Commit 3a61cca7 by Adam

Merge pull request #10195 from edx/hotfix/2015-10-15

Hotfix/2015 10 15
parents c76e2492 5ec3d83d
...@@ -681,6 +681,16 @@ class MiscCourseTests(ContentStoreTestCase): ...@@ -681,6 +681,16 @@ class MiscCourseTests(ContentStoreTestCase):
for expected in expected_types: for expected in expected_types:
self.assertIn(expected, resp.content) self.assertIn(expected, resp.content)
@ddt.data("<script>alert(1)</script>", "alert('hi')", "</script><script>alert(1)</script>")
def test_container_handler_xss_prevent(self, malicious_code):
"""
Test that XSS attack is prevented
"""
resp = self.client.get_html(get_url('container_handler', self.vert_loc) + '?action=' + malicious_code)
self.assertEqual(resp.status_code, 200)
# Test that malicious code does not appear in html
self.assertNotIn(malicious_code, resp.content)
@patch('django.conf.settings.DEPRECATED_ADVANCED_COMPONENT_TYPES', []) @patch('django.conf.settings.DEPRECATED_ADVANCED_COMPONENT_TYPES', [])
def test_advanced_components_in_edit_unit(self): def test_advanced_components_in_edit_unit(self):
# This could be made better, but for now let's just assert that we see the advanced modules mentioned in the page # This could be made better, but for now let's just assert that we see the advanced modules mentioned in the page
......
...@@ -2,6 +2,7 @@ ...@@ -2,6 +2,7 @@
Public views Public views
""" """
from django.views.decorators.csrf import ensure_csrf_cookie from django.views.decorators.csrf import ensure_csrf_cookie
from django.views.decorators.clickjacking import xframe_options_deny
from django.core.context_processors import csrf from django.core.context_processors import csrf
from django.core.urlresolvers import reverse from django.core.urlresolvers import reverse
from django.shortcuts import redirect from django.shortcuts import redirect
...@@ -17,6 +18,7 @@ __all__ = ['signup', 'login_page', 'howitworks'] ...@@ -17,6 +18,7 @@ __all__ = ['signup', 'login_page', 'howitworks']
@ensure_csrf_cookie @ensure_csrf_cookie
@xframe_options_deny
def signup(request): def signup(request):
""" """
Display the signup form. Display the signup form.
...@@ -34,6 +36,7 @@ def signup(request): ...@@ -34,6 +36,7 @@ def signup(request):
@ssl_login_shortcut @ssl_login_shortcut
@ensure_csrf_cookie @ensure_csrf_cookie
@xframe_options_deny
def login_page(request): def login_page(request):
""" """
Display the login form. Display the login form.
......
...@@ -13,28 +13,28 @@ import json ...@@ -13,28 +13,28 @@ import json
from contentstore.views.helpers import xblock_studio_url, xblock_type_display_name from contentstore.views.helpers import xblock_studio_url, xblock_type_display_name
from django.utils.translation import ugettext as _ from django.utils.translation import ugettext as _
%> %>
<%block name="title">${xblock.display_name_with_default} ${xblock_type_display_name(xblock)}</%block> <%block name="title">${xblock.display_name_with_default} ${xblock_type_display_name(xblock) | h}</%block>
<%block name="bodyclass">is-signedin course container view-container</%block> <%block name="bodyclass">is-signedin course container view-container</%block>
<%namespace name='static' file='static_content.html'/> <%namespace name='static' file='static_content.html'/>
<%block name="header_extras"> <%block name="header_extras">
% for template_name in templates: % for template_name in templates:
<script type="text/template" id="${template_name}-tpl"> <script type="text/template" id="${template_name | h}-tpl">
<%static:include path="js/${template_name}.underscore" /> <%static:include path="js/${template_name}.underscore" />
</script> </script>
% endfor % endfor
<script type="text/template" id="image-modal-tpl"> <script type="text/template" id="image-modal-tpl">
<%static:include path="common/templates/image-modal.underscore" /> <%static:include path="common/templates/image-modal.underscore" />
</script> </script>
<link rel="stylesheet" type="text/css" href="${static.url('js/vendor/timepicker/jquery.timepicker.css')}" /> <link rel="stylesheet" type="text/css" href="${static.url('js/vendor/timepicker/jquery.timepicker.css') | h}" />
</%block> </%block>
<%block name="requirejs"> <%block name="requirejs">
require(["js/factories/container"], function(ContainerFactory) { require(["js/factories/container"], function(ContainerFactory) {
ContainerFactory( ContainerFactory(
${component_templates | n}, ${json.dumps(xblock_info) | n}, ${component_templates | n}, ${json.dumps(xblock_info) | n},
"${action}", "${action | h}",
{ {
isUnitPage: ${json.dumps(is_unit_page)}, isUnitPage: ${json.dumps(is_unit_page)},
canEdit: true canEdit: true
...@@ -55,7 +55,7 @@ from django.utils.translation import ugettext as _ ...@@ -55,7 +55,7 @@ from django.utils.translation import ugettext as _
ancestor_url = xblock_studio_url(ancestor) ancestor_url = xblock_studio_url(ancestor)
%> %>
% if ancestor_url: % if ancestor_url:
<a href="${ancestor_url}" class="navigation-item navigation-link navigation-parent">${ancestor.display_name_with_default | h}</a> <a href="${ancestor_url | h}" class="navigation-item navigation-link navigation-parent">${ancestor.display_name_with_default | h}</a>
% else: % else:
<span class="navigation-item navigation-parent">${ancestor.display_name_with_default | h}</span> <span class="navigation-item navigation-parent">${ancestor.display_name_with_default | h}</span>
% endif % endif
...@@ -72,12 +72,12 @@ from django.utils.translation import ugettext as _ ...@@ -72,12 +72,12 @@ from django.utils.translation import ugettext as _
<ul> <ul>
% if is_unit_page: % if is_unit_page:
<li class="action-item action-view nav-item"> <li class="action-item action-view nav-item">
<a href="${published_preview_link}" class="button button-view action-button is-disabled" aria-disabled="true" rel="external" title="${_('Open the courseware in the LMS')}"> <a href="${published_preview_link | h}" class="button button-view action-button is-disabled" aria-disabled="true" rel="external" title="${_('Open the courseware in the LMS')}">
<span class="action-button-text">${_("View Live Version")}</span> <span class="action-button-text">${_("View Live Version")}</span>
</a> </a>
</li> </li>
<li class="action-item action-preview nav-item"> <li class="action-item action-preview nav-item">
<a href="${draft_preview_link}" class="button button-preview action-button" rel="external" title="${_('Preview the courseware in the LMS')}"> <a href="${draft_preview_link | h}" class="button button-preview action-button" rel="external" title="${_('Preview the courseware in the LMS')}">
<span class="action-button-text">${_("Preview")}</span> <span class="action-button-text">${_("Preview")}</span>
</a> </a>
</li> </li>
...@@ -110,10 +110,10 @@ from django.utils.translation import ugettext as _ ...@@ -110,10 +110,10 @@ from django.utils.translation import ugettext as _
% if xblock.category == 'split_test': % if xblock.category == 'split_test':
<div class="bit"> <div class="bit">
<h3 class="title-3">${_("Adding components")}</h3> <h3 class="title-3">${_("Adding components")}</h3>
<p>${_("Select a component type under {em_start}Add New Component{em_end}. Then select a template.").format(em_start='<strong>', em_end="</strong>")}</p> <p>${_("Select a component type under {em_start}Add New Component{em_end}. Then select a template.").format(em_start='<strong>', em_end="</strong>") | h}</p>
<p>${_("The new component is added at the bottom of the page or group. You can then edit and move the component.")}</p> <p>${_("The new component is added at the bottom of the page or group. You can then edit and move the component.")}</p>
<h3 class="title-3">${_("Editing components")}</h3> <h3 class="title-3">${_("Editing components")}</h3>
<p>${_("Click the {em_start}Edit{em_end} icon in a component to edit its content.").format(em_start='<strong>', em_end="</strong>")}</p> <p>${_("Click the {em_start}Edit{em_end} icon in a component to edit its content.").format(em_start='<strong>', em_end="</strong>") | h}</p>
<h3 class="title-3">${_("Reorganizing components")}</h3> <h3 class="title-3">${_("Reorganizing components")}</h3>
<p>${_("Drag components to new locations within this component.")}</p> <p>${_("Drag components to new locations within this component.")}</p>
<p>${_("For content experiments, you can drag components to other groups.")}</p> <p>${_("For content experiments, you can drag components to other groups.")}</p>
...@@ -121,7 +121,7 @@ from django.utils.translation import ugettext as _ ...@@ -121,7 +121,7 @@ from django.utils.translation import ugettext as _
<p>${_("Confirm that you have properly configured content in each of your experiment groups.")}</p> <p>${_("Confirm that you have properly configured content in each of your experiment groups.")}</p>
</div> </div>
<div class="bit external-help"> <div class="bit external-help">
<a href="${get_online_help_info(online_help_token())['doc_url']}" target="_blank" class="button external-help-button">${_("Learn more about component containers")}</a> <a href="${get_online_help_info(online_help_token())['doc_url'] | h}" target="_blank" class="button external-help-button">${_("Learn more about component containers")}</a>
</div> </div>
% elif is_unit_page: % elif is_unit_page:
<div id="publish-unit"></div> <div id="publish-unit"></div>
......
"""
Decorators that can be used to interact with third_party_auth.
"""
from functools import wraps
from urlparse import urlparse
from django.conf import settings
from django.utils.decorators import available_attrs
from third_party_auth.models import LTIProviderConfig
def xframe_allow_whitelisted(view_func):
"""
Modifies a view function so that its response has the X-Frame-Options HTTP header
set to 'DENY' if the request HTTP referrer is not from a whitelisted hostname.
"""
def wrapped_view(request, *args, **kwargs):
""" Modify the response with the correct X-Frame-Options. """
resp = view_func(request, *args, **kwargs)
x_frame_option = 'DENY'
if settings.FEATURES['ENABLE_THIRD_PARTY_AUTH']:
referer = request.META.get('HTTP_REFERER')
if referer is not None:
parsed_url = urlparse(referer)
hostname = parsed_url.hostname
if LTIProviderConfig.objects.current_set().filter(lti_hostname=hostname, enabled=True).exists():
x_frame_option = 'ALLOW'
resp['X-Frame-Options'] = x_frame_option
return resp
return wraps(view_func, assigned=available_attrs(view_func))(wrapped_view)
...@@ -493,6 +493,15 @@ class LTIProviderConfig(ProviderConfig): ...@@ -493,6 +493,15 @@ class LTIProviderConfig(ProviderConfig):
'The name that the LTI Tool Consumer will use to identify itself' 'The name that the LTI Tool Consumer will use to identify itself'
) )
) )
lti_hostname = models.CharField(
max_length=255,
help_text=(
'The domain that will be acting as the LTI consumer.'
),
db_index=True
)
lti_consumer_secret = models.CharField( lti_consumer_secret = models.CharField(
default=long_token, default=long_token,
max_length=255, max_length=255,
......
"""
Tests for third_party_auth decorators.
"""
import ddt
import unittest
from django.conf import settings
from django.http import HttpResponse
from django.test import RequestFactory
from third_party_auth.decorators import xframe_allow_whitelisted
from third_party_auth.tests.testutil import TestCase
@xframe_allow_whitelisted
def mock_view(_request):
""" A test view for testing purposes. """
return HttpResponse()
# remove this decorator once third_party_auth is enabled in CMS
@unittest.skipIf(
'third_party_auth' not in settings.INSTALLED_APPS,
'third_party_auth is not currently installed in CMS'
)
@ddt.ddt
class TestXFrameWhitelistDecorator(TestCase):
""" Test the xframe_allow_whitelisted decorator. """
def setUp(self):
super(TestXFrameWhitelistDecorator, self).setUp()
self.configure_lti_provider(name='Test', lti_hostname='localhost', lti_consumer_key='test_key', enabled=True)
self.factory = RequestFactory()
def construct_request(self, referer):
""" Add the given referer to a request and then return it. """
request = self.factory.get('/login')
request.META['HTTP_REFERER'] = referer
return request
@ddt.unpack
@ddt.data(
('http://localhost:8000/login', 'ALLOW'),
('http://not-a-real-domain.com/login', 'DENY'),
(None, 'DENY')
)
def test_x_frame_options(self, url, expected_result):
request = self.construct_request(url)
response = mock_view(request)
self.assertEqual(response['X-Frame-Options'], expected_result)
@ddt.data('http://localhost/login', 'http://not-a-real-domain.com', None)
def test_feature_flag_off(self, url):
with self.settings(FEATURES={'ENABLE_THIRD_PARTY_AUTH': False}):
request = self.construct_request(url)
response = mock_view(request)
self.assertEqual(response['X-Frame-Options'], 'DENY')
...@@ -46,6 +46,11 @@ class TeamCardsMixin(object): ...@@ -46,6 +46,11 @@ class TeamCardsMixin(object):
"""Return the names of each team on the page.""" """Return the names of each team on the page."""
return self.q(css=self._bounded_selector('p.card-description')).map(lambda e: e.text).results return self.q(css=self._bounded_selector('p.card-description')).map(lambda e: e.text).results
@property
def team_memberships(self):
"""Return the team memberships text for each card on the page."""
return self.q(css=self._bounded_selector('.member-count')).map(lambda e: e.text).results
class BreadcrumbsMixin(object): class BreadcrumbsMixin(object):
"""Provides common operations on teams page breadcrumb links.""" """Provides common operations on teams page breadcrumb links."""
......
...@@ -78,6 +78,18 @@ class TeamsTabBase(EventsTestMixin, UniqueCourseTest): ...@@ -78,6 +78,18 @@ class TeamsTabBase(EventsTestMixin, UniqueCourseTest):
self.assertEqual(response.status_code, 200) self.assertEqual(response.status_code, 200)
return json.loads(response.text) return json.loads(response.text)
def create_memberships(self, num_memberships, team_id):
"""Create `num_memberships` users and assign them to `team_id`. The
last user created becomes the current user."""
memberships = []
for __ in xrange(num_memberships):
user_info = AutoAuthPage(self.browser, course_id=self.course_id).visit().user_info
memberships.append(user_info)
self.create_membership(user_info['username'], team_id)
#pylint: disable=attribute-defined-outside-init
self.user_info = memberships[-1]
return memberships
def create_membership(self, username, team_id): def create_membership(self, username, team_id):
"""Assign `username` to `team_id`.""" """Assign `username` to `team_id`."""
response = self.course_fixture.session.post( response = self.course_fixture.session.post(
...@@ -339,6 +351,18 @@ class MyTeamsTest(TeamsTabBase): ...@@ -339,6 +351,18 @@ class MyTeamsTest(TeamsTabBase):
self.my_teams_page.visit() self.my_teams_page.visit()
self.verify_teams(self.my_teams_page, teams) self.verify_teams(self.my_teams_page, teams)
def test_multiple_team_members(self):
"""
Scenario: Visiting the My Teams page when user is a member of a team should display the teams.
Given I am a member of a team with multiple members
When I visit the My Teams page
Then I should see the correct number of team members on my membership
"""
teams = self.create_teams(self.topic, 1)
self.create_memberships(4, teams[0]['id'])
self.my_teams_page.visit()
self.assertEqual(self.my_teams_page.team_memberships[0], '4 / 10 Members')
@attr('shard_5') @attr('shard_5')
@ddt.ddt @ddt.ddt
......
...@@ -680,6 +680,16 @@ class ProgressPageTests(ModuleStoreTestCase): ...@@ -680,6 +680,16 @@ class ProgressPageTests(ModuleStoreTestCase):
self.section = ItemFactory.create(category='sequential', parent_location=self.chapter.location) self.section = ItemFactory.create(category='sequential', parent_location=self.chapter.location)
self.vertical = ItemFactory.create(category='vertical', parent_location=self.section.location) self.vertical = ItemFactory.create(category='vertical', parent_location=self.section.location)
@ddt.data('"><script>alert(1)</script>', '<script>alert(1)</script>', '</script><script>alert(1)</script>')
def test_progress_page_xss_prevent(self, malicious_code):
"""
Test that XSS attack is prevented
"""
resp = views.progress(self.request, course_id=unicode(self.course.id), student_id=self.user.id)
self.assertEqual(resp.status_code, 200)
# Test that malicious code does not appear in html
self.assertNotIn(malicious_code, resp.content)
def test_pure_ungraded_xblock(self): def test_pure_ungraded_xblock(self):
ItemFactory.create(category='acid', parent_location=self.vertical.location) ItemFactory.create(category='acid', parent_location=self.vertical.location)
......
...@@ -450,7 +450,7 @@ class SingleCohortedThreadTestCase(CohortedTestCase): ...@@ -450,7 +450,7 @@ class SingleCohortedThreadTestCase(CohortedTestCase):
html = response.content html = response.content
# Verify that the group name is correctly included in the HTML # Verify that the group name is correctly included in the HTML
self.assertRegexpMatches(html, r'&quot;group_name&quot;: &quot;student_cohort&quot;') self.assertRegexpMatches(html, r'&#34;group_name&#34;: &#34;student_cohort&#34;')
@patch('lms.lib.comment_client.utils.requests.request') @patch('lms.lib.comment_client.utils.requests.request')
...@@ -1152,10 +1152,10 @@ class UserProfileTestCase(ModuleStoreTestCase): ...@@ -1152,10 +1152,10 @@ class UserProfileTestCase(ModuleStoreTestCase):
self.assertRegexpMatches(html, r'data-num-pages="1"') self.assertRegexpMatches(html, r'data-num-pages="1"')
self.assertRegexpMatches(html, r'<span>1</span> discussion started') self.assertRegexpMatches(html, r'<span>1</span> discussion started')
self.assertRegexpMatches(html, r'<span>2</span> comments') self.assertRegexpMatches(html, r'<span>2</span> comments')
self.assertRegexpMatches(html, r'&quot;id&quot;: &quot;{}&quot;'.format(self.TEST_THREAD_ID)) self.assertRegexpMatches(html, r'&#34;id&#34;: &#34;{}&#34;'.format(self.TEST_THREAD_ID))
self.assertRegexpMatches(html, r'&quot;title&quot;: &quot;{}&quot;'.format(self.TEST_THREAD_TEXT)) self.assertRegexpMatches(html, r'&#34;title&#34;: &#34;{}&#34;'.format(self.TEST_THREAD_TEXT))
self.assertRegexpMatches(html, r'&quot;body&quot;: &quot;{}&quot;'.format(self.TEST_THREAD_TEXT)) self.assertRegexpMatches(html, r'&#34;body&#34;: &#34;{}&#34;'.format(self.TEST_THREAD_TEXT))
self.assertRegexpMatches(html, r'&quot;username&quot;: &quot;{}&quot;'.format(self.student.username)) self.assertRegexpMatches(html, r'&#34;username&#34;: &#34;{}&#34;'.format(self.student.username))
def check_ajax(self, mock_request, **params): def check_ajax(self, mock_request, **params):
response = self.get_response(mock_request, params, HTTP_X_REQUESTED_WITH="XMLHttpRequest") response = self.get_response(mock_request, params, HTTP_X_REQUESTED_WITH="XMLHttpRequest")
...@@ -1326,6 +1326,56 @@ class ForumFormDiscussionUnicodeTestCase(ModuleStoreTestCase, UnicodeTestMixin): ...@@ -1326,6 +1326,56 @@ class ForumFormDiscussionUnicodeTestCase(ModuleStoreTestCase, UnicodeTestMixin):
self.assertEqual(response_data["discussion_data"][0]["body"], text) self.assertEqual(response_data["discussion_data"][0]["body"], text)
@ddt.ddt
@patch('lms.lib.comment_client.utils.requests.request')
class ForumDiscussionXSSTestCase(UrlResetMixin, ModuleStoreTestCase):
@patch.dict("django.conf.settings.FEATURES", {"ENABLE_DISCUSSION_SERVICE": True})
def setUp(self):
super(ForumDiscussionXSSTestCase, self).setUp()
username = "foo"
password = "bar"
self.course = CourseFactory.create()
self.student = UserFactory.create(username=username, password=password)
CourseEnrollmentFactory.create(user=self.student, course_id=self.course.id)
self.assertTrue(self.client.login(username=username, password=password))
@ddt.data('"><script>alert(1)</script>', '<script>alert(1)</script>', '</script><script>alert(1)</script>')
@patch('student.models.cc.User.from_django_user')
def test_forum_discussion_xss_prevent(self, malicious_code, mock_user, mock_req): # pylint: disable=unused-argument
"""
Test that XSS attack is prevented
"""
reverse_url = "%s%s" % (reverse(
"django_comment_client.forum.views.forum_form_discussion",
kwargs={"course_id": unicode(self.course.id)}), '/forum_form_discussion')
# Test that malicious code does not appear in html
url = "%s?%s=%s" % (reverse_url, 'sort_key', malicious_code)
resp = self.client.get(url)
self.assertEqual(resp.status_code, 200)
self.assertNotIn(malicious_code, resp.content)
@ddt.data('"><script>alert(1)</script>', '<script>alert(1)</script>', '</script><script>alert(1)</script>')
@patch('student.models.cc.User.from_django_user')
@patch('student.models.cc.User.active_threads')
def test_forum_user_profile_xss_prevent(self, malicious_code, mock_threads, mock_from_django_user, mock_request):
"""
Test that XSS attack is prevented
"""
mock_threads.return_value = [], 1, 1
mock_from_django_user.return_value = Mock()
mock_request.side_effect = make_mock_request_impl(course=self.course, text='dummy')
url = reverse('django_comment_client.forum.views.user_profile',
kwargs={'course_id': unicode(self.course.id), 'user_id': str(self.student.id)})
# Test that malicious code does not appear in html
url_string = "%s?%s=%s" % (url, 'page', malicious_code)
resp = self.client.get(url_string)
self.assertEqual(resp.status_code, 200)
self.assertNotIn(malicious_code, resp.content)
class ForumDiscussionSearchUnicodeTestCase(ModuleStoreTestCase, UnicodeTestMixin): class ForumDiscussionSearchUnicodeTestCase(ModuleStoreTestCase, UnicodeTestMixin):
def setUp(self): def setUp(self):
super(ForumDiscussionSearchUnicodeTestCase, self).setUp() super(ForumDiscussionSearchUnicodeTestCase, self).setUp()
......
...@@ -5,7 +5,6 @@ Views handling read (GET) requests for the Discussion tab and inline discussions ...@@ -5,7 +5,6 @@ Views handling read (GET) requests for the Discussion tab and inline discussions
from functools import wraps from functools import wraps
import json import json
import logging import logging
import xml.sax.saxutils as saxutils
from django.contrib.auth.decorators import login_required from django.contrib.auth.decorators import login_required
from django.conf import settings from django.conf import settings
...@@ -73,13 +72,6 @@ class DiscussionTab(EnrolledTab): ...@@ -73,13 +72,6 @@ class DiscussionTab(EnrolledTab):
return settings.FEATURES.get('ENABLE_DISCUSSION_SERVICE') return settings.FEATURES.get('ENABLE_DISCUSSION_SERVICE')
def _attr_safe_json(obj):
"""
return a JSON string for obj which is safe to embed as the value of an attribute in a DOM node
"""
return saxutils.escape(json.dumps(obj), {'"': '&quot;'})
@newrelic.agent.function_trace() @newrelic.agent.function_trace()
def make_course_settings(course, user): def make_course_settings(course, user):
""" """
...@@ -278,28 +270,28 @@ def forum_form_discussion(request, course_key): ...@@ -278,28 +270,28 @@ def forum_form_discussion(request, course_key):
'course': course, 'course': course,
#'recent_active_threads': recent_active_threads, #'recent_active_threads': recent_active_threads,
'staff_access': bool(has_access(request.user, 'staff', course)), 'staff_access': bool(has_access(request.user, 'staff', course)),
'threads': _attr_safe_json(threads), 'threads': json.dumps(threads),
'thread_pages': query_params['num_pages'], 'thread_pages': query_params['num_pages'],
'user_info': _attr_safe_json(user_info), 'user_info': json.dumps(user_info, default=lambda x: None),
'can_create_comment': _attr_safe_json( 'can_create_comment': json.dumps(
has_permission(request.user, "create_comment", course.id)), has_permission(request.user, "create_comment", course.id)),
'can_create_subcomment': _attr_safe_json( 'can_create_subcomment': json.dumps(
has_permission(request.user, "create_sub_comment", course.id)), has_permission(request.user, "create_sub_comment", course.id)),
'can_create_thread': has_permission(request.user, "create_thread", course.id), 'can_create_thread': has_permission(request.user, "create_thread", course.id),
'flag_moderator': bool( 'flag_moderator': bool(
has_permission(request.user, 'openclose_thread', course.id) or has_permission(request.user, 'openclose_thread', course.id) or
has_access(request.user, 'staff', course) has_access(request.user, 'staff', course)
), ),
'annotated_content_info': _attr_safe_json(annotated_content_info), 'annotated_content_info': json.dumps(annotated_content_info),
'course_id': course.id.to_deprecated_string(), 'course_id': course.id.to_deprecated_string(),
'roles': _attr_safe_json(utils.get_role_ids(course_key)), 'roles': json.dumps(utils.get_role_ids(course_key)),
'is_moderator': has_permission(request.user, "see_all_cohorts", course_key), 'is_moderator': has_permission(request.user, "see_all_cohorts", course_key),
'cohorts': course_settings["cohorts"], # still needed to render _thread_list_template 'cohorts': course_settings["cohorts"], # still needed to render _thread_list_template
'user_cohort': user_cohort_id, # read from container in NewPostView 'user_cohort': user_cohort_id, # read from container in NewPostView
'is_course_cohorted': is_course_cohorted(course_key), # still needed to render _thread_list_template 'is_course_cohorted': is_course_cohorted(course_key), # still needed to render _thread_list_template
'sort_preference': user.default_sort_key, 'sort_preference': user.default_sort_key,
'category_map': course_settings["category_map"], 'category_map': course_settings["category_map"],
'course_settings': _attr_safe_json(course_settings) 'course_settings': json.dumps(course_settings)
} }
# print "start rendering.." # print "start rendering.."
return render_to_response('discussion/index.html', context) return render_to_response('discussion/index.html', context)
...@@ -385,19 +377,19 @@ def single_thread(request, course_key, discussion_id, thread_id): ...@@ -385,19 +377,19 @@ def single_thread(request, course_key, discussion_id, thread_id):
'discussion_id': discussion_id, 'discussion_id': discussion_id,
'csrf': csrf(request)['csrf_token'], 'csrf': csrf(request)['csrf_token'],
'init': '', # TODO: What is this? 'init': '', # TODO: What is this?
'user_info': _attr_safe_json(user_info), 'user_info': json.dumps(user_info),
'can_create_comment': _attr_safe_json( 'can_create_comment': json.dumps(
has_permission(request.user, "create_comment", course.id)), has_permission(request.user, "create_comment", course.id)),
'can_create_subcomment': _attr_safe_json( 'can_create_subcomment': json.dumps(
has_permission(request.user, "create_sub_comment", course.id)), has_permission(request.user, "create_sub_comment", course.id)),
'can_create_thread': has_permission(request.user, "create_thread", course.id), 'can_create_thread': has_permission(request.user, "create_thread", course.id),
'annotated_content_info': _attr_safe_json(annotated_content_info), 'annotated_content_info': json.dumps(annotated_content_info),
'course': course, 'course': course,
#'recent_active_threads': recent_active_threads, #'recent_active_threads': recent_active_threads,
'course_id': course.id.to_deprecated_string(), # TODO: Why pass both course and course.id to template? 'course_id': course.id.to_deprecated_string(), # TODO: Why pass both course and course.id to template?
'thread_id': thread_id, 'thread_id': thread_id,
'threads': _attr_safe_json(threads), 'threads': json.dumps(threads),
'roles': _attr_safe_json(utils.get_role_ids(course_key)), 'roles': json.dumps(utils.get_role_ids(course_key)),
'is_moderator': is_moderator, 'is_moderator': is_moderator,
'thread_pages': query_params['num_pages'], 'thread_pages': query_params['num_pages'],
'is_course_cohorted': is_course_cohorted(course_key), 'is_course_cohorted': is_course_cohorted(course_key),
...@@ -409,7 +401,7 @@ def single_thread(request, course_key, discussion_id, thread_id): ...@@ -409,7 +401,7 @@ def single_thread(request, course_key, discussion_id, thread_id):
'user_cohort': user_cohort, 'user_cohort': user_cohort,
'sort_preference': cc_user.default_sort_key, 'sort_preference': cc_user.default_sort_key,
'category_map': course_settings["category_map"], 'category_map': course_settings["category_map"],
'course_settings': _attr_safe_json(course_settings) 'course_settings': json.dumps(course_settings)
} }
return render_to_response('discussion/index.html', context) return render_to_response('discussion/index.html', context)
...@@ -458,7 +450,7 @@ def user_profile(request, course_key, user_id): ...@@ -458,7 +450,7 @@ def user_profile(request, course_key, user_id):
'discussion_data': threads, 'discussion_data': threads,
'page': query_params['page'], 'page': query_params['page'],
'num_pages': query_params['num_pages'], 'num_pages': query_params['num_pages'],
'annotated_content_info': _attr_safe_json(annotated_content_info), 'annotated_content_info': json.dumps(annotated_content_info),
}) })
else: else:
django_user = User.objects.get(id=user_id) django_user = User.objects.get(id=user_id)
...@@ -467,9 +459,9 @@ def user_profile(request, course_key, user_id): ...@@ -467,9 +459,9 @@ def user_profile(request, course_key, user_id):
'user': request.user, 'user': request.user,
'django_user': django_user, 'django_user': django_user,
'profiled_user': profiled_user.to_dict(), 'profiled_user': profiled_user.to_dict(),
'threads': _attr_safe_json(threads), 'threads': json.dumps(threads),
'user_info': _attr_safe_json(user_info), 'user_info': json.dumps(user_info, default=lambda x: None),
'annotated_content_info': _attr_safe_json(annotated_content_info), 'annotated_content_info': json.dumps(annotated_content_info),
'page': query_params['page'], 'page': query_params['page'],
'num_pages': query_params['num_pages'], 'num_pages': query_params['num_pages'],
'learner_profile_page_url': reverse('learner_profile', kwargs={'username': django_user.username}) 'learner_profile_page_url': reverse('learner_profile', kwargs={'username': django_user.username})
...@@ -546,9 +538,9 @@ def followed_threads(request, course_key, user_id): ...@@ -546,9 +538,9 @@ def followed_threads(request, course_key, user_id):
'user': request.user, 'user': request.user,
'django_user': User.objects.get(id=user_id), 'django_user': User.objects.get(id=user_id),
'profiled_user': profiled_user.to_dict(), 'profiled_user': profiled_user.to_dict(),
'threads': _attr_safe_json(threads), 'threads': json.dumps(threads),
'user_info': _attr_safe_json(user_info), 'user_info': json.dumps(user_info),
'annotated_content_info': _attr_safe_json(annotated_content_info), 'annotated_content_info': json.dumps(annotated_content_info),
# 'content': content, # 'content': content,
} }
......
...@@ -24,10 +24,10 @@ class GroupIdAssertionMixin(object): ...@@ -24,10 +24,10 @@ class GroupIdAssertionMixin(object):
def _assert_html_response_contains_group_info(self, response): def _assert_html_response_contains_group_info(self, response):
group_info = {"group_id": None, "group_name": None} group_info = {"group_id": None, "group_name": None}
match = re.search(r'&quot;group_id&quot;: ([\d]*)', response.content) match = re.search(r'&#34;group_id&#34;: ([\d]*)', response.content)
if match and match.group(1) != '': if match and match.group(1) != '':
group_info["group_id"] = int(match.group(1)) group_info["group_id"] = int(match.group(1))
match = re.search(r'&quot;group_name&quot;: &quot;([^&]*)&quot;', response.content) match = re.search(r'&#34;group_name&#34;: &#34;([^&]*)&#34;', response.content)
if match: if match:
group_info["group_name"] = match.group(1) group_info["group_name"] = match.group(1)
self._assert_thread_contains_group_info(group_info) self._assert_thread_contains_group_info(group_info)
......
...@@ -354,6 +354,24 @@ class StudentAccountLoginAndRegistrationTest(ThirdPartyAuthTestMixin, UrlResetMi ...@@ -354,6 +354,24 @@ class StudentAccountLoginAndRegistrationTest(ThirdPartyAuthTestMixin, UrlResetMi
self.assertContains(resp, "Register for Test Microsite") self.assertContains(resp, "Register for Test Microsite")
self.assertContains(resp, "register-form") self.assertContains(resp, "register-form")
def test_login_registration_xframe_protected(self):
resp = self.client.get(
reverse("register_user"),
{},
HTTP_REFERER="http://localhost/iframe"
)
self.assertEqual(resp['X-Frame-Options'], 'DENY')
self.configure_lti_provider(name='Test', lti_hostname='localhost', lti_consumer_key='test_key', enabled=True)
resp = self.client.get(
reverse("register_user"),
HTTP_REFERER="http://localhost/iframe"
)
self.assertEqual(resp['X-Frame-Options'], 'ALLOW')
def _assert_third_party_auth_data(self, response, current_backend, current_provider, providers): def _assert_third_party_auth_data(self, response, current_backend, current_provider, providers):
"""Verify that third party auth info is rendered correctly in a DOM data attribute. """ """Verify that third party auth info is rendered correctly in a DOM data attribute. """
finish_auth_url = None finish_auth_url = None
......
...@@ -34,6 +34,7 @@ from student.views import ( ...@@ -34,6 +34,7 @@ from student.views import (
from student.helpers import get_next_url_for_login_page from student.helpers import get_next_url_for_login_page
import third_party_auth import third_party_auth
from third_party_auth import pipeline from third_party_auth import pipeline
from third_party_auth.decorators import xframe_allow_whitelisted
from util.bad_request_rate_limiter import BadRequestRateLimiter from util.bad_request_rate_limiter import BadRequestRateLimiter
from openedx.core.djangoapps.user_api.accounts.api import request_password_change from openedx.core.djangoapps.user_api.accounts.api import request_password_change
...@@ -45,6 +46,7 @@ AUDIT_LOG = logging.getLogger("audit") ...@@ -45,6 +46,7 @@ AUDIT_LOG = logging.getLogger("audit")
@require_http_methods(['GET']) @require_http_methods(['GET'])
@ensure_csrf_cookie @ensure_csrf_cookie
@xframe_allow_whitelisted
def login_and_registration_form(request, initial_mode="login"): def login_and_registration_form(request, initial_mode="login"):
"""Render the combined login/registration form, defaulting to login """Render the combined login/registration form, defaulting to login
......
;(function (define) {
'use strict';
define(['teams/js/collections/team'], function (TeamCollection) {
var MyTeamsCollection = TeamCollection.extend({
initialize: function (teams, options) {
TeamCollection.prototype.initialize.call(this, teams, options);
delete this.server_api.topic_id;
this.server_api = _.extend(this.server_api, {
username: options.username
});
}
});
return MyTeamsCollection;
});
}).call(this, define || RequireJS.define);
...@@ -9,8 +9,6 @@ ...@@ -9,8 +9,6 @@
this.perPage = options.per_page || 10; this.perPage = options.per_page || 10;
this.username = options.username; this.username = options.username;
this.privileged = options.privileged;
this.staff = options.staff;
this.server_api = _.extend( this.server_api = _.extend(
{ {
...@@ -24,15 +22,7 @@ ...@@ -24,15 +22,7 @@
delete this.server_api['order_by']; // Order by is not specified for the TeamMembership API delete this.server_api['order_by']; // Order by is not specified for the TeamMembership API
}, },
model: TeamMembershipModel, model: TeamMembershipModel
canUserCreateTeam: function() {
// Note: non-staff and non-privileged users are automatically added to any team
// that they create. This means that if multiple team membership is
// disabled that they cannot create a new team when they already
// belong to one.
return this.privileged || this.staff || this.length === 0;
}
}); });
return TeamMembershipCollection; return TeamMembershipCollection;
}); });
......
define([ define([
'backbone', 'backbone',
'teams/js/collections/team', 'teams/js/collections/my_teams',
'teams/js/collections/team_membership',
'teams/js/views/my_teams', 'teams/js/views/my_teams',
'teams/js/spec_helpers/team_spec_helpers', 'teams/js/spec_helpers/team_spec_helpers',
'common/js/spec_helpers/ajax_helpers' 'common/js/spec_helpers/ajax_helpers'
], function (Backbone, TeamCollection, TeamMembershipCollection, MyTeamsView, TeamSpecHelpers, AjaxHelpers) { ], function (Backbone, MyTeamsCollection, MyTeamsView, TeamSpecHelpers, AjaxHelpers) {
'use strict'; 'use strict';
describe('My Teams View', function () { describe('My Teams View', function () {
beforeEach(function () { beforeEach(function () {
setFixtures('<div class="teams-container"></div>'); setFixtures('<div class="teams-container"></div>');
}); });
var createMyTeamsView = function(options) { var createMyTeamsView = function(myTeams) {
return new MyTeamsView(_.extend( return new MyTeamsView({
{ el: '.teams-container',
el: '.teams-container', collection: myTeams,
collection: options.teams || TeamSpecHelpers.createMockTeams(), showActions: true,
teamMemberships: TeamSpecHelpers.createMockTeamMemberships(), context: TeamSpecHelpers.testContext
showActions: true, }).render();
context: TeamSpecHelpers.testContext
},
options
)).render();
}; };
it('can render itself', function () { it('can render itself', function () {
var teamMembershipsData = TeamSpecHelpers.createMockTeamMembershipsData(1, 5), var teamsData = TeamSpecHelpers.createMockTeamData(1, 5),
teamMemberships = TeamSpecHelpers.createMockTeamMemberships(teamMembershipsData), teams = TeamSpecHelpers.createMockTeams({results: teamsData}),
myTeamsView = createMyTeamsView({ myTeamsView = createMyTeamsView(teams);
teams: teamMemberships, TeamSpecHelpers.verifyCards(myTeamsView, teamsData);
teamMemberships: teamMemberships
});
TeamSpecHelpers.verifyCards(myTeamsView, teamMembershipsData);
// Verify that there is no header or footer // Verify that there is no header or footer
expect(myTeamsView.$('.teams-paging-header').text().trim()).toBe(''); expect(myTeamsView.$('.teams-paging-header').text().trim()).toBe('');
...@@ -41,36 +32,37 @@ define([ ...@@ -41,36 +32,37 @@ define([
}); });
it('shows a message when the user is not a member of any teams', function () { it('shows a message when the user is not a member of any teams', function () {
var teamMemberships = TeamSpecHelpers.createMockTeamMemberships([]), var teams = TeamSpecHelpers.createMockTeams({results: []}),
myTeamsView = createMyTeamsView({ myTeamsView = createMyTeamsView(teams);
teams: teamMemberships,
teamMemberships: teamMemberships
});
TeamSpecHelpers.verifyCards(myTeamsView, []); TeamSpecHelpers.verifyCards(myTeamsView, []);
expect(myTeamsView.$el.text().trim()).toBe('You are not currently a member of any team.'); expect(myTeamsView.$el.text().trim()).toBe('You are not currently a member of any team.');
}); });
it('refreshes a stale membership collection when rendering', function() { it('refreshes a stale membership collection when rendering', function() {
var requests = AjaxHelpers.requests(this), var requests = AjaxHelpers.requests(this),
teamMemberships = TeamSpecHelpers.createMockTeamMemberships([]), teams = TeamSpecHelpers.createMockTeams({
myTeamsView = createMyTeamsView({ results: []
teams: teamMemberships, }, {
teamMemberships: teamMemberships per_page: 2,
}); url: TeamSpecHelpers.testContext.myTeamsUrl,
username: TeamSpecHelpers.testContext.userInfo.username
}, MyTeamsCollection),
myTeamsView = createMyTeamsView(teams);
TeamSpecHelpers.verifyCards(myTeamsView, []); TeamSpecHelpers.verifyCards(myTeamsView, []);
expect(myTeamsView.$el.text().trim()).toBe('You are not currently a member of any team.'); expect(myTeamsView.$el.text().trim()).toBe('You are not currently a member of any team.');
teamMemberships.teamEvents.trigger('teams:update', { action: 'create' }); TeamSpecHelpers.teamEvents.trigger('teams:update', { action: 'create' });
myTeamsView.render(); myTeamsView.render();
AjaxHelpers.expectRequestURL( AjaxHelpers.expectRequestURL(
requests, requests,
TeamSpecHelpers.testContext.teamMembershipsUrl, TeamSpecHelpers.testContext.myTeamsUrl,
{ {
expand : 'team,user', expand : 'user',
username : TeamSpecHelpers.testContext.userInfo.username, username : TeamSpecHelpers.testContext.userInfo.username,
course_id : TeamSpecHelpers.testContext.courseID, course_id : TeamSpecHelpers.testContext.courseID,
page : '1', page : '1',
page_size : '10', page_size : '2',
text_search: '' text_search: '',
order_by: 'last_activity_at'
} }
); );
AjaxHelpers.respondWithJson(requests, {}); AjaxHelpers.respondWithJson(requests, {});
......
define([ define([
'backbone', 'backbone',
'teams/js/collections/team', 'teams/js/collections/team',
'teams/js/collections/team_membership',
'teams/js/views/teams', 'teams/js/views/teams',
'teams/js/spec_helpers/team_spec_helpers' 'teams/js/spec_helpers/team_spec_helpers'
], function (Backbone, TeamCollection, TeamMembershipCollection, TeamsView, TeamSpecHelpers) { ], function (Backbone, TeamCollection, TeamsView, TeamSpecHelpers) {
'use strict'; 'use strict';
describe('Teams View', function () { describe('Teams View', function () {
beforeEach(function () { beforeEach(function () {
...@@ -15,7 +14,6 @@ define([ ...@@ -15,7 +14,6 @@ define([
return new TeamsView({ return new TeamsView({
el: '.teams-container', el: '.teams-container',
collection: options.teams || TeamSpecHelpers.createMockTeams(), collection: options.teams || TeamSpecHelpers.createMockTeams(),
teamMemberships: options.teamMemberships || TeamSpecHelpers.createMockTeamMemberships(),
showActions: true, showActions: true,
context: TeamSpecHelpers.testContext context: TeamSpecHelpers.testContext
}).render(); }).render();
......
define([ define([
'backbone', 'backbone',
'teams/js/collections/team', 'underscore',
'teams/js/collections/team_membership',
'teams/js/views/topic_teams', 'teams/js/views/topic_teams',
'teams/js/spec_helpers/team_spec_helpers', 'teams/js/spec_helpers/team_spec_helpers',
'common/js/spec_helpers/ajax_helpers',
'common/js/spec_helpers/page_helpers' 'common/js/spec_helpers/page_helpers'
], function (Backbone, TeamCollection, TeamMembershipCollection, TopicTeamsView, TeamSpecHelpers, ], function (Backbone, _, TopicTeamsView, TeamSpecHelpers, PageHelpers) {
AjaxHelpers, PageHelpers) {
'use strict'; 'use strict';
describe('Topic Teams View', function () { describe('Topic Teams View', function () {
var createTopicTeamsView = function(options) { var createTopicTeamsView = function(options) {
options = options || {};
var myTeamsCollection = options.myTeamsCollection || TeamSpecHelpers.createMockTeams({results: []});
return new TopicTeamsView({ return new TopicTeamsView({
el: '.teams-container', el: '.teams-container',
model: TeamSpecHelpers.createMockTopic(), model: TeamSpecHelpers.createMockTopic(),
collection: options.teams || TeamSpecHelpers.createMockTeams(), collection: options.teams || TeamSpecHelpers.createMockTeams(),
teamMemberships: options.teamMemberships || TeamSpecHelpers.createMockTeamMemberships(), myTeamsCollection: myTeamsCollection,
showActions: true, showActions: true,
context: TeamSpecHelpers.testContext context: _.extend({}, TeamSpecHelpers.testContext, options)
}).render(); }).render();
}; };
...@@ -49,8 +48,7 @@ define([ ...@@ -49,8 +48,7 @@ define([
teamsView = createTopicTeamsView({ teamsView = createTopicTeamsView({
teams: TeamSpecHelpers.createMockTeams({ teams: TeamSpecHelpers.createMockTeams({
results: testTeamData results: testTeamData
}), })
teamMemberships: TeamSpecHelpers.createMockTeamMemberships([])
}); });
expect(teamsView.$('.teams-paging-header').text()).toMatch('Showing 1-5 out of 6 total'); expect(teamsView.$('.teams-paging-header').text()).toMatch('Showing 1-5 out of 6 total');
...@@ -64,24 +62,21 @@ define([ ...@@ -64,24 +62,21 @@ define([
}); });
it('can browse all teams', function () { it('can browse all teams', function () {
var emptyMembership = TeamSpecHelpers.createMockTeamMemberships([]), var teamsView = createTopicTeamsView();
teamsView = createTopicTeamsView({ teamMemberships: emptyMembership });
spyOn(Backbone.history, 'navigate'); spyOn(Backbone.history, 'navigate');
teamsView.$('.browse-teams').click(); teamsView.$('.browse-teams').click();
expect(Backbone.history.navigate.calls[0].args).toContain('browse'); expect(Backbone.history.navigate.calls[0].args).toContain('browse');
}); });
it('gives the search field focus when clicking on the search teams link', function () { it('gives the search field focus when clicking on the search teams link', function () {
var emptyMembership = TeamSpecHelpers.createMockTeamMemberships([]), var teamsView = createTopicTeamsView();
teamsView = createTopicTeamsView({ teamMemberships: emptyMembership });
spyOn($.fn, 'focus').andCallThrough(); spyOn($.fn, 'focus').andCallThrough();
teamsView.$('.search-teams').click(); teamsView.$('.search-teams').click();
expect(teamsView.$('.search-field').first().focus).toHaveBeenCalled(); expect(teamsView.$('.search-field').first().focus).toHaveBeenCalled();
}); });
it('can show the create team modal', function () { it('can show the create team modal', function () {
var emptyMembership = TeamSpecHelpers.createMockTeamMemberships([]), var teamsView = createTopicTeamsView();
teamsView = createTopicTeamsView({ teamMemberships: emptyMembership });
spyOn(Backbone.history, 'navigate'); spyOn(Backbone.history, 'navigate');
teamsView.$('a.create-team').click(); teamsView.$('a.create-team').click();
expect(Backbone.history.navigate.calls[0].args).toContain( expect(Backbone.history.navigate.calls[0].args).toContain(
...@@ -90,25 +85,17 @@ define([ ...@@ -90,25 +85,17 @@ define([
}); });
it('does not show actions for a user already in a team', function () { it('does not show actions for a user already in a team', function () {
var teamsView = createTopicTeamsView({}); var teamsView = createTopicTeamsView({myTeamsCollection: TeamSpecHelpers.createMockTeams()});
verifyActions(teamsView, {showActions: false}); verifyActions(teamsView, {showActions: false});
}); });
it('shows actions for a privileged user already in a team', function () { it('shows actions for a privileged user already in a team', function () {
var staffMembership = TeamSpecHelpers.createMockTeamMemberships( var teamsView = createTopicTeamsView({ privileged: true });
TeamSpecHelpers.createMockTeamMembershipsData(1, 5),
{ privileged: true }
),
teamsView = createTopicTeamsView({ teamMemberships: staffMembership });
verifyActions(teamsView); verifyActions(teamsView);
}); });
it('shows actions for a staff user already in a team', function () { it('shows actions for a staff user already in a team', function () {
var staffMembership = TeamSpecHelpers.createMockTeamMemberships( var teamsView = createTopicTeamsView({ privileged: false, staff: true });
TeamSpecHelpers.createMockTeamMembershipsData(1, 5),
{ privileged: false, staff: true }
),
teamsView = createTopicTeamsView({ teamMemberships: staffMembership });
verifyActions(teamsView); verifyActions(teamsView);
}); });
......
...@@ -56,14 +56,18 @@ define([ ...@@ -56,14 +56,18 @@ define([
); );
}; };
var createMockTeams = function(options) { var createMockTeams = function(responseOptions, options, collectionType) {
return new TeamCollection( if(_.isUndefined(collectionType)) {
createMockTeamsResponse(options), collectionType = TeamCollection;
{ }
return new collectionType(
createMockTeamsResponse(responseOptions),
_.extend({
teamEvents: teamEvents, teamEvents: teamEvents,
course_id: testCourseID, course_id: testCourseID,
per_page: 2,
parse: true parse: true
} }, options)
); );
}; };
...@@ -83,35 +87,6 @@ define([ ...@@ -83,35 +87,6 @@ define([
}); });
}; };
var createMockTeamMemberships = function(teamMembershipData, options) {
if (!teamMembershipData) {
teamMembershipData = createMockTeamMembershipsData(1, 5);
}
return new TeamMembershipCollection(
{
count: 11,
num_pages: 3,
current_page: 1,
start: 0,
sort_order: 'last_activity_at',
results: teamMembershipData
},
_.extend(
{},
{
teamEvents: teamEvents,
course_id: testCourseID,
parse: true,
url: testContext.teamMembershipsUrl,
username: testUser,
privileged: false,
staff: false
},
options
)
);
};
var createMockUserInfo = function(options) { var createMockUserInfo = function(options) {
return _.extend( return _.extend(
{ {
...@@ -291,6 +266,7 @@ define([ ...@@ -291,6 +266,7 @@ define([
teamsDetailUrl: '/api/team/v0/teams/team_id', teamsDetailUrl: '/api/team/v0/teams/team_id',
teamMembershipsUrl: '/api/team/v0/team_memberships/', teamMembershipsUrl: '/api/team/v0/team_memberships/',
teamMembershipDetailUrl: '/api/team/v0/team_membership/team_id,' + testUser, teamMembershipDetailUrl: '/api/team/v0/team_membership/team_id,' + testUser,
myTeamsUrl: '/api/team/v0/teams/',
userInfo: createMockUserInfo() userInfo: createMockUserInfo()
}; };
...@@ -331,8 +307,6 @@ define([ ...@@ -331,8 +307,6 @@ define([
createMockTeamData: createMockTeamData, createMockTeamData: createMockTeamData,
createMockTeamsResponse: createMockTeamsResponse, createMockTeamsResponse: createMockTeamsResponse,
createMockTeams: createMockTeams, createMockTeams: createMockTeams,
createMockTeamMembershipsData: createMockTeamMembershipsData,
createMockTeamMemberships: createMockTeamMemberships,
createMockUserInfo: createMockUserInfo, createMockUserInfo: createMockUserInfo,
createMockContext: createMockContext, createMockContext: createMockContext,
createMockTopic: createMockTopic, createMockTopic: createMockTopic,
......
...@@ -22,12 +22,11 @@ ...@@ -22,12 +22,11 @@
}, },
initialize: function(options) { initialize: function(options) {
this.teamMembershipDetailUrl = options.context.teamMembershipDetailUrl;
// The URL ends with team_id,request_username. We want to replace // The URL ends with team_id,request_username. We want to replace
// the last occurrence of team_id with the actual team_id, and remove request_username // the last occurrence of team_id with the actual team_id, and remove request_username
// as the actual user to be removed from the team will be added on before calling DELETE. // as the actual user to be removed from the team will be added on before calling DELETE.
this.teamMembershipDetailUrl = this.teamMembershipDetailUrl.substring( this.teamMembershipDetailUrl = options.context.teamMembershipDetailUrl.substring(
0, this.teamMembershipDetailUrl.lastIndexOf('team_id') 0, this.options.context.teamMembershipDetailUrl.lastIndexOf('team_id')
) + this.model.get('id') + ","; ) + this.model.get('id') + ",";
this.teamEvents = options.teamEvents; this.teamEvents = options.teamEvents;
......
;(function (define) { (function (define) {
'use strict'; 'use strict';
define([ define([
'jquery', 'jquery',
...@@ -99,48 +99,34 @@ ...@@ -99,48 +99,34 @@
CardView.prototype.initialize.apply(this, arguments); CardView.prototype.initialize.apply(this, arguments);
// TODO: show last activity detail view // TODO: show last activity detail view
this.detailViews = [ this.detailViews = [
new TeamMembershipView({memberships: this.getMemberships(), maxTeamSize: this.maxTeamSize}), new TeamMembershipView({memberships: this.model.get('membership'), maxTeamSize: this.maxTeamSize}),
new TeamCountryLanguageView({ new TeamCountryLanguageView({
model: this.teamModel(), model: this.model,
countries: this.countries, countries: this.countries,
languages: this.languages languages: this.languages
}), }),
new TeamActivityView({date: this.teamModel().get('last_activity_at')}) new TeamActivityView({date: this.model.get('last_activity_at')})
]; ];
this.model.on('change:membership', function () { this.model.on('change:membership', function () {
this.detailViews[0].memberships = this.getMemberships(); this.detailViews[0].memberships = this.model.get('membership');
}, this); }, this);
}, },
teamModel: function () {
if (this.model.has('team')) { return this.model.get('team'); }
return this.model;
},
getMemberships: function () {
if (this.model.has('team')) {
return [this.model.attributes];
}
else {
return this.model.get('membership');
}
},
configuration: 'list_card', configuration: 'list_card',
cardClass: 'team-card', cardClass: 'team-card',
title: function () { return this.teamModel().get('name'); }, title: function () { return this.model.get('name'); },
description: function () { return this.teamModel().get('description'); }, description: function () { return this.model.get('description'); },
details: function () { return this.detailViews; }, details: function () { return this.detailViews; },
actionClass: 'action-view', actionClass: 'action-view',
actionContent: function() { actionContent: function() {
return interpolate( return interpolate(
gettext('View %(span_start)s %(team_name)s %(span_end)s'), gettext('View %(span_start)s %(team_name)s %(span_end)s'),
{span_start: '<span class="sr">', team_name: _.escape(this.teamModel().get('name')), span_end: '</span>'}, {span_start: '<span class="sr">', team_name: _.escape(this.model.get('name')), span_end: '</span>'},
true true
); );
}, },
actionUrl: function () { actionUrl: function () {
return '#teams/' + this.teamModel().get('topic_id') + '/' + this.teamModel().get('id'); return '#teams/' + this.model.get('topic_id') + '/' + this.model.get('id');
} }
}); });
return TeamCardView; return TeamCardView;
......
...@@ -16,12 +16,9 @@ ...@@ -16,12 +16,9 @@
}, },
initialize: function (options) { initialize: function (options) {
this.topic = options.topic;
this.teamMemberships = options.teamMemberships;
this.context = options.context; this.context = options.context;
this.itemViewClass = TeamCardView.extend({ this.itemViewClass = TeamCardView.extend({
router: options.router, router: options.router,
topic: options.topic,
maxTeamSize: this.context.maxTeamSize, maxTeamSize: this.context.maxTeamSize,
srInfo: this.srInfo, srInfo: this.srInfo,
countries: TeamUtils.selectorOptionsArrayToHashWithBlank(this.context.countries), countries: TeamUtils.selectorOptionsArrayToHashWithBlank(this.context.countries),
......
...@@ -12,7 +12,7 @@ ...@@ -12,7 +12,7 @@
'teams/js/collections/topic', 'teams/js/collections/topic',
'teams/js/models/team', 'teams/js/models/team',
'teams/js/collections/team', 'teams/js/collections/team',
'teams/js/collections/team_membership', 'teams/js/collections/my_teams',
'teams/js/utils/team_analytics', 'teams/js/utils/team_analytics',
'teams/js/views/teams_tabbed_view', 'teams/js/views/teams_tabbed_view',
'teams/js/views/topics', 'teams/js/views/topics',
...@@ -26,7 +26,7 @@ ...@@ -26,7 +26,7 @@
'teams/js/views/instructor_tools', 'teams/js/views/instructor_tools',
'text!teams/templates/teams_tab.underscore'], 'text!teams/templates/teams_tab.underscore'],
function (Backbone, $, _, gettext, SearchFieldView, HeaderView, HeaderModel, function (Backbone, $, _, gettext, SearchFieldView, HeaderView, HeaderModel,
TopicModel, TopicCollection, TeamModel, TeamCollection, TeamMembershipCollection, TeamAnalytics, TopicModel, TopicCollection, TeamModel, TeamCollection, MyTeamsCollection, TeamAnalytics,
TeamsTabbedView, TopicsView, TeamProfileView, MyTeamsView, TopicTeamsView, TeamEditView, TeamsTabbedView, TopicsView, TeamProfileView, MyTeamsView, TopicTeamsView, TeamEditView,
TeamMembersEditView, TeamProfileHeaderActionsView, TeamUtils, InstructorToolsView, teamsTemplate) { TeamMembersEditView, TeamProfileHeaderActionsView, TeamUtils, InstructorToolsView, teamsTemplate) {
var TeamsHeaderModel = HeaderModel.extend({ var TeamsHeaderModel = HeaderModel.extend({
...@@ -95,26 +95,22 @@ ...@@ -95,26 +95,22 @@
// Create an event queue to track team changes // Create an event queue to track team changes
this.teamEvents = _.clone(Backbone.Events); this.teamEvents = _.clone(Backbone.Events);
this.myTeamsCollection = new MyTeamsCollection(
this.teamMemberships = new TeamMembershipCollection( this.context.userInfo.teams,
this.context.userInfo.team_memberships_data,
{ {
teamEvents: this.teamEvents, teamEvents: this.teamEvents,
url: this.context.teamMembershipsUrl,
course_id: this.context.courseID, course_id: this.context.courseID,
username: this.context.userInfo.username, username: this.context.userInfo.username,
privileged: this.context.userInfo.privileged, per_page: 2,
staff: this.context.userInfo.staff, parse: true,
parse: true url: this.context.myTeamsUrl
} }
).bootstrap(); );
this.myTeamsView = new MyTeamsView({ this.myTeamsView = new MyTeamsView({
router: this.router, router: this.router,
teamEvents: this.teamEvents, teamEvents: this.teamEvents,
context: this.context, context: this.context,
collection: this.teamMemberships, collection: this.myTeamsCollection
teamMemberships: this.teamMemberships
}); });
this.topicsCollection = new TopicCollection( this.topicsCollection = new TopicCollection(
...@@ -176,7 +172,7 @@ ...@@ -176,7 +172,7 @@
// 1. If the user belongs to at least one team, jump to the "My Teams" page // 1. If the user belongs to at least one team, jump to the "My Teams" page
// 2. If not, then jump to the "Browse" page // 2. If not, then jump to the "Browse" page
if (Backbone.history.getFragment() === '') { if (Backbone.history.getFragment() === '') {
if (this.teamMemberships.length > 0) { if (this.myTeamsCollection.length > 0) {
this.router.navigate('my-teams', {trigger: true}); this.router.navigate('my-teams', {trigger: true});
} else { } else {
this.router.navigate('browse', {trigger: true}); this.router.navigate('browse', {trigger: true});
...@@ -351,12 +347,13 @@ ...@@ -351,12 +347,13 @@
createTeamsListView: function(options) { createTeamsListView: function(options) {
var topic = options.topic, var topic = options.topic,
collection = options.collection, collection = options.collection,
self = this,
teamsView = new TopicTeamsView({ teamsView = new TopicTeamsView({
router: this.router, router: this.router,
context: this.context, context: this.context,
model: topic, model: topic,
collection: collection, collection: collection,
teamMemberships: this.teamMemberships, myTeamsCollection: this.myTeamsCollection,
showSortControls: options.showSortControls showSortControls: options.showSortControls
}), }),
searchFieldView = new SearchFieldView({ searchFieldView = new SearchFieldView({
......
...@@ -16,38 +16,44 @@ ...@@ -16,38 +16,44 @@
initialize: function(options) { initialize: function(options) {
this.showSortControls = options.showSortControls; this.showSortControls = options.showSortControls;
this.context = options.context;
this.myTeamsCollection = options.myTeamsCollection;
TeamsView.prototype.initialize.call(this, options); TeamsView.prototype.initialize.call(this, options);
}, },
canUserCreateTeam: function () {
// Note: non-staff and non-privileged users are automatically added to any team
// that they create. This means that if multiple team membership is
// disabled that they cannot create a new team when they already
// belong to one.
return this.context.staff || this.context.privileged || this.myTeamsCollection.length === 0;
},
render: function() { render: function() {
var self = this; var self = this;
$.when( this.collection.refresh().done(function() {
this.collection.refresh(), TeamsView.prototype.render.call(self);
this.teamMemberships.refresh() if (self.canUserCreateTeam()) {
).done(function() { var message = interpolate_text(
TeamsView.prototype.render.call(self); // Translators: this string is shown at the bottom of the teams page
// to find a team to join or else to create a new one. There are three
if (self.teamMemberships.canUserCreateTeam()) { // links that need to be included in the message:
var message = interpolate_text( // 1. Browse teams in other topics
// Translators: this string is shown at the bottom of the teams page // 2. search teams
// to find a team to join or else to create a new one. There are three // 3. create a new team
// links that need to be included in the message: // Be careful to start each link with the appropriate start indicator
// 1. Browse teams in other topics // (e.g. {browse_span_start} for #1) and finish it with {span_end}.
// 2. search teams _.escape(gettext("{browse_span_start}Browse teams in other topics{span_end} or {search_span_start}search teams{span_end} in this topic. If you still can't find a team to join, {create_span_start}create a new team in this topic{span_end}.")),
// 3. create a new team {
// Be careful to start each link with the appropriate start indicator 'browse_span_start': '<a class="browse-teams" href="">',
// (e.g. {browse_span_start} for #1) and finish it with {span_end}. 'search_span_start': '<a class="search-teams" href="">',
_.escape(gettext("{browse_span_start}Browse teams in other topics{span_end} or {search_span_start}search teams{span_end} in this topic. If you still can't find a team to join, {create_span_start}create a new team in this topic{span_end}.")), 'create_span_start': '<a class="create-team" href="">',
{ 'span_end': '</a>'
'browse_span_start': '<a class="browse-teams" href="">', }
'search_span_start': '<a class="search-teams" href="">', );
'create_span_start': '<a class="create-team" href="">', self.$el.append(_.template(teamActionsTemplate, {message: message}));
'span_end': '</a>' }
} });
);
self.$el.append(_.template(teamActionsTemplate, {message: message}));
}
});
return this; return this;
}, },
...@@ -68,7 +74,10 @@ ...@@ -68,7 +74,10 @@
showCreateTeamForm: function (event) { showCreateTeamForm: function (event) {
event.preventDefault(); event.preventDefault();
Backbone.history.navigate('topics/' + this.model.id + '/create-team', {trigger: true}); Backbone.history.navigate(
'topics/' + this.model.id + '/create-team',
{trigger: true}
);
}, },
createHeaderView: function () { createHeaderView: function () {
......
...@@ -42,6 +42,7 @@ ...@@ -42,6 +42,7 @@
teamsDetailUrl: '${ teams_detail_url }', teamsDetailUrl: '${ teams_detail_url }',
teamMembershipsUrl: '${ team_memberships_url }', teamMembershipsUrl: '${ team_memberships_url }',
teamMembershipDetailUrl: '${ team_membership_detail_url }', teamMembershipDetailUrl: '${ team_membership_detail_url }',
myTeamsUrl: '${ my_teams_url }',
maxTeamSize: ${ course.teams_max_size }, maxTeamSize: ${ course.teams_max_size },
languages: ${ json.dumps(languages, cls=EscapedEdxJSONEncoder) }, languages: ${ json.dumps(languages, cls=EscapedEdxJSONEncoder) },
countries: ${ json.dumps(countries, cls=EscapedEdxJSONEncoder) }, countries: ${ json.dumps(countries, cls=EscapedEdxJSONEncoder) },
......
...@@ -507,6 +507,10 @@ class TestListTeamsAPI(EventTestMixin, TeamAPITestCase): ...@@ -507,6 +507,10 @@ class TestListTeamsAPI(EventTestMixin, TeamAPITestCase):
def test_filter_topic_id(self): def test_filter_topic_id(self):
self.verify_names({'course_id': self.test_course_1.id, 'topic_id': 'topic_0'}, 200, [u'Sólar team']) self.verify_names({'course_id': self.test_course_1.id, 'topic_id': 'topic_0'}, 200, [u'Sólar team'])
def test_filter_username(self):
self.verify_names({'course_id': self.test_course_1.id, 'username': 'student_enrolled'}, 200, [u'Sólar team'])
self.verify_names({'course_id': self.test_course_1.id, 'username': 'staff'}, 200, [])
@ddt.data( @ddt.data(
(None, 200, ['Nuclear Team', u'Sólar team', 'Wind Team']), (None, 200, ['Nuclear Team', u'Sólar team', 'Wind Team']),
('name', 200, ['Nuclear Team', u'Sólar team', 'Wind Team']), ('name', 200, ['Nuclear Team', u'Sólar team', 'Wind Team']),
......
...@@ -107,8 +107,8 @@ class TopicsPagination(TeamAPIPagination): ...@@ -107,8 +107,8 @@ class TopicsPagination(TeamAPIPagination):
page_size = TOPICS_PER_PAGE page_size = TOPICS_PER_PAGE
class MembershipPagination(TeamAPIPagination): class MyTeamsPagination(TeamAPIPagination):
"""Paginate memberships. """ """Paginate the user's teams. """
page_size = TEAM_MEMBERSHIPS_PER_PAGE page_size = TEAM_MEMBERSHIPS_PER_PAGE
...@@ -153,14 +153,15 @@ class TeamsDashboardView(GenericAPIView): ...@@ -153,14 +153,15 @@ class TeamsDashboardView(GenericAPIView):
) )
topics_data["sort_order"] = sort_order topics_data["sort_order"] = sort_order
# Paginate and serialize team membership data. user = request.user
team_memberships = CourseTeamMembership.get_memberships(user.username, [course.id])
memberships_data = self._serialize_and_paginate( user_teams = CourseTeam.objects.filter(membership__user=user)
MembershipPagination, user_teams_data = self._serialize_and_paginate(
team_memberships, MyTeamsPagination,
user_teams,
request, request,
MembershipSerializer, CourseTeamSerializer,
{'expand': ('team', 'user',)} {'expand': ('user',)}
) )
context = { context = {
...@@ -173,7 +174,7 @@ class TeamsDashboardView(GenericAPIView): ...@@ -173,7 +174,7 @@ class TeamsDashboardView(GenericAPIView):
"username": user.username, "username": user.username,
"privileged": has_discussion_privileges(user, course_key), "privileged": has_discussion_privileges(user, course_key),
"staff": bool(has_access(user, 'staff', course_key)), "staff": bool(has_access(user, 'staff', course_key)),
"team_memberships_data": memberships_data, "teams": user_teams_data
}, },
"topic_url": reverse( "topic_url": reverse(
'topics_detail', kwargs={'topic_id': 'topic_id', 'course_id': str(course_id)}, request=request 'topics_detail', kwargs={'topic_id': 'topic_id', 'course_id': str(course_id)}, request=request
...@@ -182,6 +183,7 @@ class TeamsDashboardView(GenericAPIView): ...@@ -182,6 +183,7 @@ class TeamsDashboardView(GenericAPIView):
"teams_url": reverse('teams_list', request=request), "teams_url": reverse('teams_list', request=request),
"teams_detail_url": reverse('teams_detail', args=['team_id']), "teams_detail_url": reverse('teams_detail', args=['team_id']),
"team_memberships_url": reverse('team_membership_list', request=request), "team_memberships_url": reverse('team_membership_list', request=request),
"my_teams_url": reverse('teams_list', request=request),
"team_membership_detail_url": reverse('team_membership_detail', args=['team_id', user.username]), "team_membership_detail_url": reverse('team_membership_detail', args=['team_id', user.username]),
"languages": [[lang[0], _(lang[1])] for lang in settings.ALL_LANGUAGES], # pylint: disable=translation-of-non-string "languages": [[lang[0], _(lang[1])] for lang in settings.ALL_LANGUAGES], # pylint: disable=translation-of-non-string
"countries": list(countries), "countries": list(countries),
...@@ -283,6 +285,8 @@ class TeamsListView(ExpandableFieldViewMixin, GenericAPIView): ...@@ -283,6 +285,8 @@ class TeamsListView(ExpandableFieldViewMixin, GenericAPIView):
* last_activity_at: Orders result by team activity, with most active first * last_activity_at: Orders result by team activity, with most active first
(for tie-breaking, open_slots is used, with most open slots first). (for tie-breaking, open_slots is used, with most open slots first).
* username: Return teams whose membership contains the given user.
* page_size: Number of results to return per page. * page_size: Number of results to return per page.
* page: Page number to retrieve. * page: Page number to retrieve.
...@@ -414,6 +418,10 @@ class TeamsListView(ExpandableFieldViewMixin, GenericAPIView): ...@@ -414,6 +418,10 @@ class TeamsListView(ExpandableFieldViewMixin, GenericAPIView):
status=status.HTTP_400_BAD_REQUEST status=status.HTTP_400_BAD_REQUEST
) )
username = request.QUERY_PARAMS.get('username', None)
if username is not None:
result_filter.update({'membership__user__username': username})
topic_id = request.QUERY_PARAMS.get('topic_id', None) topic_id = request.QUERY_PARAMS.get('topic_id', None)
if topic_id is not None: if topic_id is not None:
if topic_id not in [topic['id'] for topic in course_module.teams_configuration['topics']]: if topic_id not in [topic['id'] for topic in course_module.teams_configuration['topics']]:
......
...@@ -25,20 +25,20 @@ from django.core.urlresolvers import reverse ...@@ -25,20 +25,20 @@ from django.core.urlresolvers import reverse
<%include file="_discussion_course_navigation.html" args="active_page='discussion'" /> <%include file="_discussion_course_navigation.html" args="active_page='discussion'" />
<section class="discussion container" id="discussion-container" <section class="discussion container" id="discussion-container"
data-roles="${roles}" data-roles="${roles | h}"
data-course-id="${course_id | h}" data-course-id="${course_id | h}"
data-course-name="${course.display_name_with_default}" data-course-name="${course.display_name_with_default | h}"
data-user-info="${user_info}" data-user-info="${user_info | h}"
data-user-create-comment="${can_create_comment}" data-user-create-comment="${can_create_comment | h}"
data-user-create-subcomment="${can_create_subcomment}" data-user-create-subcomment="${can_create_subcomment | h}"
data-read-only="false" data-read-only="false"
data-threads="${threads}" data-threads="${threads | h}"
data-thread-pages="${thread_pages}" data-thread-pages="${thread_pages | h}"
data-content-info="${annotated_content_info}" data-content-info="${annotated_content_info | h}"
data-sort-preference="${sort_preference}" data-sort-preference="${sort_preference | h}"
data-flag-moderator="${flag_moderator}" data-flag-moderator="${flag_moderator | h}"
data-user-cohort-id="${user_cohort}" data-user-cohort-id="${user_cohort | h}"
data-course-settings="${course_settings}"> data-course-settings="${course_settings | h}">
<div class="discussion-body"> <div class="discussion-body">
<div class="forum-nav" role="complementary" aria-label="${_("Discussion thread list")}"></div> <div class="forum-nav" role="complementary" aria-label="${_("Discussion thread list")}"></div>
<div class="discussion-column" role="main" aria-label="Discussion" id="discussion-column"> <div class="discussion-column" role="main" aria-label="Discussion" id="discussion-column">
......
...@@ -33,8 +33,7 @@ from django.template.defaultfilters import escapejs ...@@ -33,8 +33,7 @@ from django.template.defaultfilters import escapejs
</nav> </nav>
</section> </section>
<section class="course-content container discussion-user-threads" data-course-id="${course.id | h}" data-course-name="${course.display_name_with_default | h}" data-threads="${threads | h}" data-user-info="${user_info | h}" data-page="${page | h}" data-num-pages="${num_pages | h}"/>
<section class="course-content container discussion-user-threads" data-course-id="${course.id | h}" data-course-name="${course.display_name_with_default}" data-threads="${threads}" data-user-info="${user_info}" data-page="${page}" data-num-pages="${num_pages}"/>
</div> </div>
</section> </section>
......
<%doc>
Yet, installing google tag manager for microsite(s).
So intentionally left it blank
</%doc>
...@@ -103,6 +103,7 @@ from branding import api as branding_api ...@@ -103,6 +103,7 @@ from branding import api as branding_api
google_analytics_file = microsite.get_template_path('google_analytics.html') google_analytics_file = microsite.get_template_path('google_analytics.html')
style_overrides_file = microsite.get_value('css_overrides_file') style_overrides_file = microsite.get_value('css_overrides_file')
google_tag_manager_file = microsite.get_template_path('google_tag_manager.html')
%> %>
% if header_extra_file: % if header_extra_file:
...@@ -124,6 +125,7 @@ from branding import api as branding_api ...@@ -124,6 +125,7 @@ from branding import api as branding_api
</head> </head>
<body class="${static.dir_rtl()} <%block name='bodyclass'/> lang_${LANGUAGE_CODE}"> <body class="${static.dir_rtl()} <%block name='bodyclass'/> lang_${LANGUAGE_CODE}">
<%include file="${google_tag_manager_file}" />
<div id="page-prompt"></div> <div id="page-prompt"></div>
% if not disable_window_wrap: % if not disable_window_wrap:
<div class="window-wrap" dir="${static.dir_rtl()}"> <div class="window-wrap" dir="${static.dir_rtl()}">
......
...@@ -2,7 +2,7 @@ ...@@ -2,7 +2,7 @@
# For Harvard courses: # For Harvard courses:
-e git+https://github.com/gsehub/xblock-mentoring.git@4d1cce78dc232d5da6ffd73817b5c490e87a6eee#egg=xblock-mentoring -e git+https://github.com/gsehub/xblock-mentoring.git@4d1cce78dc232d5da6ffd73817b5c490e87a6eee#egg=xblock-mentoring
-e git+https://github.com/open-craft/problem-builder.git@859df4155c0031b5a70e7f7e9744b67b3ed331d7#egg=xblock-problem-builder -e git+https://github.com/open-craft/problem-builder.git@1cb40ca523502ca2a8a2abe5aef4d1b6735cb5c7#egg=xblock-problem-builder
# Prototype XBlocks from edX learning sciences limited roll-outs and user testing. # Prototype XBlocks from edX learning sciences limited roll-outs and user testing.
# Concept XBlock, in particular, is nowhere near finished and an early prototype. # Concept XBlock, in particular, is nowhere near finished and an early prototype.
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment