From c5912ea36791e907874d0d9f0137a92b1f08ba5c Mon Sep 17 00:00:00 2001 From: Zia Fazal Date: Thu, 12 Nov 2015 19:30:42 +0500 Subject: [PATCH 01/10] refactored code as described in SOL-1386 further code refactoring and added test fixed tests and internationalisation issues added translators comment and pulled text to same line changes based on feedback updated doctoring to make it more unstandable further optimizations --- .../certificates/tests/factories.py | 15 +- .../certificates/tests/test_views.py | 42 +- .../certificates/tests/test_webview_views.py | 113 +++- lms/djangoapps/certificates/views/webview.py | 628 ++++++++++-------- 4 files changed, 475 insertions(+), 323 deletions(-) diff --git a/lms/djangoapps/certificates/tests/factories.py b/lms/djangoapps/certificates/tests/factories.py index 599d97e5f5..eb185c50e4 100644 --- a/lms/djangoapps/certificates/tests/factories.py +++ b/lms/djangoapps/certificates/tests/factories.py @@ -38,6 +38,11 @@ class BadgeAssertionFactory(DjangoModelFactory): model = BadgeAssertion mode = 'honor' + data = { + 'image': 'http://www.example.com/image.png', + 'json': {'id': 'http://www.example.com/assertion.json'}, + 'issuer': 'http://www.example.com/issuer.json', + } class BadgeImageConfigurationFactory(DjangoModelFactory): @@ -75,7 +80,8 @@ class CertificateHtmlViewConfigurationFactory(DjangoModelFactory): }, "honor": { "certificate_type": "Honor Code", - "certificate_title": "Certificate of Achievement" + "certificate_title": "Certificate of Achievement", + "logo_url": "http://www.edx.org/honor_logo.png" }, "verified": { "certificate_type": "Verified", @@ -84,6 +90,13 @@ class CertificateHtmlViewConfigurationFactory(DjangoModelFactory): "xseries": { "certificate_title": "XSeries Certificate of Achievement", "certificate_type": "XSeries" + }, + "microsites": { + "testmicrosite": { + "company_about_url": "http://www.testmicrosite.org/about-us", + "company_privacy_url": "http://www.testmicrosite.org/edx-privacy-policy", + "company_tos_url": "http://www.testmicrosite.org/edx-terms-service" + } } }""" diff --git a/lms/djangoapps/certificates/tests/test_views.py b/lms/djangoapps/certificates/tests/test_views.py index d513975735..7c1c73f118 100644 --- a/lms/djangoapps/certificates/tests/test_views.py +++ b/lms/djangoapps/certificates/tests/test_views.py @@ -187,16 +187,6 @@ class UpdateExampleCertificateViewTest(TestCase): self.assertEqual(content['return_code'], 0) -def fakemicrosite(name, default=None): - """ - This is a test mocking function to return a microsite configuration - """ - if name == 'microsite_config_key': - return 'test_microsite' - else: - return default - - @attr('shard_1') class MicrositeCertificatesViewsTests(ModuleStoreTestCase): """ @@ -270,7 +260,6 @@ class MicrositeCertificatesViewsTests(ModuleStoreTestCase): self.course.save() self.store.update_item(self.course, self.user.id) - @patch("microsite_configuration.microsite.get_value", fakemicrosite) @override_settings(FEATURES=FEATURES_WITH_CERTS_ENABLED) def test_html_view_for_microsite(self): test_configuration_string = """{ @@ -285,18 +274,20 @@ class MicrositeCertificatesViewsTests(ModuleStoreTestCase): "logo_src": "/static/certificates/images/logo-edx.svg", "logo_url": "http://www.edx.org" }, - "test_microsite": { - "accomplishment_class_append": "accomplishment-certificate", - "platform_name": "platform_microsite", - "company_about_url": "http://www.microsite.org/about-us", - "company_privacy_url": "http://www.microsite.org/edx-privacy-policy", - "company_tos_url": "http://www.microsite.org/microsite-terms-service", - "company_verified_certificate_url": "http://www.microsite.org/verified-certificate", - "document_stylesheet_url_application": "/static/certificates/sass/main-ltr.css", - "logo_src": "/static/certificates/images/logo-microsite.svg", - "logo_url": "http://www.microsite.org", - "company_about_description": "This is special microsite aware company_about_description content", - "company_about_title": "Microsite title" + "microsites": { + "testmicrosite": { + "accomplishment_class_append": "accomplishment-certificate", + "platform_name": "platform_microsite", + "company_about_url": "http://www.microsite.org/about-us", + "company_privacy_url": "http://www.microsite.org/edx-privacy-policy", + "company_tos_url": "http://www.microsite.org/microsite-terms-service", + "company_verified_certificate_url": "http://www.microsite.org/verified-certificate", + "document_stylesheet_url_application": "/static/certificates/sass/main-ltr.css", + "logo_src": "/static/certificates/images/logo-microsite.svg", + "logo_url": "http://www.microsite.org", + "company_about_description": "This is special microsite aware company_about_description content", + "company_about_title": "Microsite title" + } }, "honor": { "certificate_type": "Honor Code" @@ -310,13 +301,12 @@ class MicrositeCertificatesViewsTests(ModuleStoreTestCase): course_id=unicode(self.course.id) ) self._add_course_certificates(count=1, signatory_count=2) - response = self.client.get(test_url) + response = self.client.get(test_url, HTTP_HOST=settings.MICROSITE_TEST_HOSTNAME) self.assertIn('platform_microsite', response.content) self.assertIn('http://www.microsite.org', response.content) self.assertIn('This is special microsite aware company_about_description content', response.content) self.assertIn('Microsite title', response.content) - @patch("microsite_configuration.microsite.get_value", fakemicrosite) @override_settings(FEATURES=FEATURES_WITH_CERTS_ENABLED) def test_html_view_microsite_configuration_missing(self): test_configuration_string = """{ @@ -343,7 +333,7 @@ class MicrositeCertificatesViewsTests(ModuleStoreTestCase): course_id=unicode(self.course.id) ) self._add_course_certificates(count=1, signatory_count=2) - response = self.client.get(test_url) + response = self.client.get(test_url, HTTP_HOST=settings.MICROSITE_TEST_HOSTNAME) self.assertIn('edX', response.content) self.assertNotIn('platform_microsite', response.content) self.assertNotIn('http://www.microsite.org', response.content) diff --git a/lms/djangoapps/certificates/tests/test_webview_views.py b/lms/djangoapps/certificates/tests/test_webview_views.py index 362122e276..76d799cbec 100644 --- a/lms/djangoapps/certificates/tests/test_webview_views.py +++ b/lms/djangoapps/certificates/tests/test_webview_views.py @@ -23,7 +23,6 @@ from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase from certificates.api import get_certificate_url from certificates.models import ( GeneratedCertificate, - BadgeAssertion, CertificateStatuses, CertificateSocialNetworks, CertificateTemplate, @@ -33,6 +32,7 @@ from certificates.models import ( from certificates.tests.factories import ( CertificateHtmlViewConfigurationFactory, LinkedInAddToProfileConfigurationFactory, + BadgeAssertionFactory, ) from util import organizations_helpers as organizations_api from django.test.client import RequestFactory @@ -221,6 +221,104 @@ class CertificatesViewsTests(ModuleStoreTestCase, EventTrackingTestCase): ) self.assertIn('logo_test1.png', response.content) + @override_settings(FEATURES=FEATURES_WITH_CERTS_ENABLED) + @patch.dict("django.conf.settings.SOCIAL_SHARING_SETTINGS", { + "CERTIFICATE_TWITTER": True, + "CERTIFICATE_FACEBOOK": True, + }) + def test_rendering_maximum_data(self): + """ + Tests at least one data item from different context update methods to + make sure every context update method is invoked while rendering certificate template. + """ + long_org_name = 'Long org name' + short_org_name = 'short_org_name' + test_organization_data = { + 'name': long_org_name, + 'short_name': short_org_name, + 'description': 'Test Organization Description', + 'active': True, + 'logo': '/logo_test1.png' + } + test_org = organizations_api.add_organization(organization_data=test_organization_data) + organizations_api.add_organization_course(organization_data=test_org, course_id=unicode(self.course.id)) + self._add_course_certificates(count=1, signatory_count=1, is_active=True) + BadgeAssertionFactory.create( + user=self.user, course_id=self.course_id, + ) + self.course.cert_html_view_overrides = { + "logo_src": "/static/certificates/images/course_override_logo.png" + } + + self.course.save() + self.store.update_item(self.course, self.user.id) + + test_url = get_certificate_url( + user_id=self.user.id, + course_id=unicode(self.course.id) + ) + response = self.client.get(test_url, HTTP_HOST=settings.MICROSITE_TEST_HOSTNAME) + + # Test an item from basic info + self.assertIn( + 'Terms of Service & Honor Code', + response.content + ) + self.assertIn( + 'Certificate ID Number', + response.content + ) + # Test an item from html cert configuration + self.assertIn( + ' verifying your identity.").format( platform_name=platform_name, tos_url=context.get('company_tos_url'), - verified_cert_url=context.get('company_verified_certificate_url') + verified_cert_url=context.get('company_verified_certificate_url')) + + +def _update_context_with_basic_info(context, course_id, platform_name, configuration): + """ + Updates context dictionary with basic info required before rendering simplest + certificate templates. + """ + context['platform_name'] = platform_name + context['course_id'] = course_id + + # Update the view context with the default ConfigurationModel settings + context.update(configuration.get('default', {})) + + # Translators: 'All rights reserved' is a legal term used in copyrighting to protect published content + reserved = _("All rights reserved") + context['copyright_text'] = '© {year} {platform_name}. {reserved}.'.format( + year=settings.COPYRIGHT_YEAR, + platform_name=platform_name, + reserved=reserved + ) + + # Translators: This text is bound to the HTML 'title' element of the page and appears + # in the browser title bar when a requested certificate is not found or recognized + context['document_title'] = _("Invalid Certificate") + + # Translators: The & characters represent an ampersand character and can be ignored + context['company_tos_urltext'] = _("Terms of Service & Honor Code") + + # Translators: A 'Privacy Policy' is a legal document/statement describing a website's use of personal information + context['company_privacy_urltext'] = _("Privacy Policy") + + # Translators: This line appears as a byline to a header image and describes the purpose of the page + context['logo_subtitle'] = _("Certificate Validation") + + # Translators: Accomplishments describe the awards/certifications obtained by students on this platform + context['accomplishment_copy_about'] = _('About {platform_name} Accomplishments').format( + platform_name=platform_name + ) + + # Translators: This line appears on the page just before the generation date for the certificate + context['certificate_date_issued_title'] = _("Issued On:") + + # Translators: The Certificate ID Number is an alphanumeric value unique to each individual certificate + context['certificate_id_number_title'] = _('Certificate ID Number') + + context['certificate_info_title'] = _('About {platform_name} Certificates').format( + platform_name=platform_name ) context['certificate_verify_title'] = _("How {platform_name} Validates Student Certificates").format( @@ -218,8 +204,7 @@ def _update_certificate_context(context, course, user, user_certificate): "world's best universities, including MIT, Harvard, Berkeley, University " "of Texas, and many others. {platform_name} is a non-profit online " "initiative created by founding partners Harvard and MIT.").format( - platform_name=platform_name - ) + platform_name=platform_name) context['company_about_title'] = _("About {platform_name}").format(platform_name=platform_name) @@ -236,187 +221,59 @@ def _update_certificate_context(context, course, user, user_certificate): platform_name=platform_name ) - # Translators: This text represents the verification of the certificate - context['document_meta_description'] = _('This is a valid {platform_name} certificate for {user_name}, ' - 'who participated in {partner_short_name} {course_number}').format( - platform_name=platform_name, - user_name=user_fullname, - partner_short_name=partner_short_name, - course_number=course_number - ) - # Translators: This text is bound to the HTML 'title' element of the page and appears in the browser title bar - context['document_title'] = _("{partner_short_name} {course_number} Certificate | {platform_name}").format( - partner_short_name=partner_short_name, - course_number=course_number, - platform_name=platform_name - ) - - # Translators: This text fragment appears after the student's name (displayed in a large font) on the certificate - # screen. The text describes the accomplishment represented by the certificate information displayed to the user - context['accomplishment_copy_description_full'] = _("successfully completed, received a passing grade, and was " - "awarded a {platform_name} {certificate_type} " - "Certificate of Completion in ").format( - platform_name=platform_name, - certificate_type=context.get("certificate_type") - ) - - certificate_type_description = get_certificate_description(user_certificate.mode, certificate_type, platform_name) - if certificate_type_description: - context['certificate_type_description'] = certificate_type_description - - # Translators: This line is displayed to a user who has completed a course and achieved a certification - context['accomplishment_banner_opening'] = _("{fullname}, you've earned a certificate!").format( - fullname=user_fullname - ) - - # Translators: This line congratulates the user and instructs them to share their accomplishment on social networks - context['accomplishment_banner_congrats'] = _("Congratulations! This page summarizes all of the details of what " - "you've accomplished. Show it off to family, friends, and colleagues " - "in your social and professional networks.") - - # Translators: This line leads the reader to understand more about the certificate that a student has been awarded - context['accomplishment_copy_more_about'] = _("More about {fullname}'s accomplishment").format( - fullname=user_fullname - ) - - -@handle_500( - template_path="certificates/server-error.html", - test_func=lambda request: request.GET.get('preview', None) -) -def render_html_view(request, user_id, course_id): +def _update_course_context(request, context, course, platform_name): """ - This public view generates an HTML representation of the specified student's certificate - If a certificate is not available, we display a "Sorry!" screen instead + Updates context dictionary with course info. """ - - # Create the initial view context, bootstrapping with Django settings and passed-in values - context = {} - context['platform_name'] = microsite.get_value("platform_name", settings.PLATFORM_NAME) - context['course_id'] = course_id - preview_mode = request.GET.get('preview', None) - - # Update the view context with the default ConfigurationModel settings - configuration = CertificateHtmlViewConfiguration.get_config() - # if we are in a microsite, then let's first see if there is an override - # section in our config - config_key = microsite.get_value('microsite_config_key', 'default') - # if there is no special microsite override, then let's use default - if config_key not in configuration: - config_key = 'default' - context.update(configuration.get(config_key, {})) - - # Translators: 'All rights reserved' is a legal term used in copyrighting to protect published content - reserved = _("All rights reserved") - context['copyright_text'] = '© {year} {platform_name}. {reserved}.'.format( - year=settings.COPYRIGHT_YEAR, - platform_name=context.get('platform_name'), - reserved=reserved - ) - - # Translators: This text is bound to the HTML 'title' element of the page and appears - # in the browser title bar when a requested certificate is not found or recognized - context['document_title'] = _("Invalid Certificate") - - # Translators: The & characters represent an ampersand character and can be ignored - context['company_tos_urltext'] = _("Terms of Service & Honor Code") - - # Translators: A 'Privacy Policy' is a legal document/statement describing a website's use of personal information - context['company_privacy_urltext'] = _("Privacy Policy") - - # Translators: This line appears as a byline to a header image and describes the purpose of the page - context['logo_subtitle'] = _("Certificate Validation") - invalid_template_path = 'certificates/invalid.html' - - # Kick the user back to the "Invalid" screen if the feature is disabled - if not has_html_certificates_enabled(course_id): - return render_to_response(invalid_template_path, context) - - # Load the core building blocks for the view context - try: - course_key = CourseKey.from_string(course_id) - user = User.objects.get(id=user_id) - course = modulestore().get_course(course_key) - - if not course: - raise CourseDoesNotExist - - # Attempt to load the user's generated certificate data - if preview_mode: - user_certificate = GeneratedCertificate.objects.get( - user=user, - course_id=course_key, - mode=preview_mode - ) - else: - user_certificate = GeneratedCertificate.objects.get( - user=user, - course_id=course_key - ) - - # If there's no generated certificate data for this user, we need to see if we're in 'preview' mode... - # If we are, we'll need to create a mock version of the user_certificate container for previewing - except GeneratedCertificate.DoesNotExist: - if preview_mode and ( - has_access(request.user, 'instructor', course) - or has_access(request.user, 'staff', course) - ): - user_certificate = GeneratedCertificate( - mode=preview_mode, - verify_uuid=unicode(uuid4().hex), - modified_date=datetime.now().date() - ) - else: - return render_to_response(invalid_template_path, context) - - # For any other expected exceptions, kick the user back to the "Invalid" screen - except (InvalidKeyError, CourseDoesNotExist, User.DoesNotExist): - return render_to_response(invalid_template_path, context) - - # Badge Request Event Tracking Logic - if 'evidence_visit' in request.GET: - try: - badge = BadgeAssertion.objects.get(user=user, course_id=course_key) - tracker.emit( - 'edx.badge.assertion.evidence_visited', - { - 'user_id': user.id, - 'course_id': unicode(course_key), - 'enrollment_mode': badge.mode, - 'assertion_id': badge.id, - 'assertion_image_url': badge.data['image'], - 'assertion_json_url': badge.data['json']['id'], - 'issuer': badge.data['issuer'], - } - ) - except BadgeAssertion.DoesNotExist: - log.warn( - "Could not find badge for %s on course %s.", - user.id, - course_key, - ) - - # Okay, now we have all of the pieces, time to put everything together - - # Get the active certificate configuration for this course - # If we do not have an active certificate, we'll need to send the user to the "Invalid" screen - # Passing in the 'preview' parameter, if specified, will return a configuration, if defined - active_configuration = get_active_web_certificate(course, preview_mode) - if active_configuration is None: - return render_to_response(invalid_template_path, context) + context['full_course_image_url'] = request.build_absolute_uri(course_image_url(course)) + course_title_from_cert = context['certificate_data'].get('course_title', '') + accomplishment_copy_course_name = course_title_from_cert if course_title_from_cert else course.display_name + context['accomplishment_copy_course_name'] = accomplishment_copy_course_name + course_number = course.display_coursenumber if course.display_coursenumber else course.number + context['course_number'] = course_number + if context['organization_long_name']: + # Translators: This text represents the description of course + context['accomplishment_copy_course_description'] = _('a course of study offered by {partner_short_name}, ' + 'an online learning initiative of {partner_long_name} ' + 'through {platform_name}.').format( + partner_short_name=context['organization_short_name'], + partner_long_name=context['organization_long_name'], + platform_name=platform_name) else: - context['certificate_data'] = active_configuration + # Translators: This text represents the description of course + context['accomplishment_copy_course_description'] = _('a course of study offered by {partner_short_name}, ' + 'through {platform_name}.').format( + partner_short_name=context['organization_short_name'], + platform_name=platform_name) - # Append/Override the existing view context values with any mode-specific ConfigurationModel values - context.update(configuration.get(user_certificate.mode, {})) - # Append/Override the existing view context values with request-time values - _update_certificate_context(context, course, user, user_certificate) +def _update_social_context(request, context, course, user, user_certificate, platform_name): + """ + Updates context dictionary with info required for social sharing. + """ + share_settings = getattr(settings, 'SOCIAL_SHARING_SETTINGS', {}) + context['facebook_share_enabled'] = share_settings.get('CERTIFICATE_FACEBOOK', False) + context['facebook_app_id'] = getattr(settings, "FACEBOOK_APP_ID", None) + context['facebook_share_text'] = share_settings.get( + 'CERTIFICATE_FACEBOOK_TEXT', + _("I completed the {course_title} course on {platform_name}.").format( + course_title=context['accomplishment_copy_course_name'], + platform_name=platform_name + ) + ) + context['twitter_share_enabled'] = share_settings.get('CERTIFICATE_TWITTER', False) + context['twitter_share_text'] = share_settings.get( + 'CERTIFICATE_TWITTER_TEXT', + _("I completed a course on {platform_name}. Take a look at my certificate.").format( + platform_name=platform_name + ) + ) + share_url = request.build_absolute_uri( reverse( 'certificates:html_view', - kwargs=dict(user_id=str(user_id), course_id=unicode(course_id)) + kwargs=dict(user_id=str(user.id), course_id=unicode(course.id)) ) ) context['share_url'] = share_url @@ -427,8 +284,7 @@ def render_html_view(request, user_id, course_id): share_url=urllib.quote_plus(smart_str(share_url)) ) context['twitter_url'] = twitter_url - context['full_course_image_url'] = request.build_absolute_uri(course_image_url(course)) - + context['linked_in_url'] = None # If enabled, show the LinkedIn "add to profile" button # Clicking this button sends the user to LinkedIn where they # can add the certificate information to their profile. @@ -446,39 +302,110 @@ def render_html_view(request, user_id, course_id): course_id=unicode(course.id) ))) ) - else: - context['linked_in_url'] = None - # Microsites will need to be able to override any hard coded - # content that was put into the context in the - # _update_certificate_context() call above. For example the - # 'company_about_description' talks about edX, which we most likely - # do not want to keep in a microsite - # - # So we need to re-apply any configuration/content that - # we are sourceing from the database. This is somewhat duplicative of - # the code at the beginning of this method, but we - # need the configuration at the top as some error code paths - # require that to be set up early on in the pipeline - # - microsite_config_key = microsite.get_value('microsite_config_key') - if microsite_config_key: - context.update(configuration.get(microsite_config_key, {})) + +def _update_context_with_user_info(context, user, user_certificate): + """ + Updates context dictionary with user related info. + """ + user_fullname = user.profile.name + context['username'] = user.username + context['course_mode'] = user_certificate.mode + context['accomplishment_user_id'] = user.id + context['accomplishment_copy_name'] = user_fullname + context['accomplishment_copy_username'] = user.username + + context['accomplishment_more_title'] = _("More Information About {user_name}'s Certificate:").format( + user_name=user_fullname + ) + # Translators: This line is displayed to a user who has completed a course and achieved a certification + context['accomplishment_banner_opening'] = _("{fullname}, you've earned a certificate!").format( + fullname=user_fullname + ) + + # Translators: This line congratulates the user and instructs them to share their accomplishment on social networks + context['accomplishment_banner_congrats'] = _("Congratulations! This page summarizes all of the details of what " + "you've accomplished. Show it off to family, friends, and colleagues " + "in your social and professional networks.") + + # Translators: This line leads the reader to understand more about the certificate that a student has been awarded + context['accomplishment_copy_more_about'] = _("More about {fullname}'s accomplishment").format( + fullname=user_fullname + ) + + +def _get_user_certificate(request, user, course_key, course, preview_mode=None): + """ + Retrieves user's certificate from db. Creates one in case of preview mode. + Returns None if there is no certificate generated for given user + otherwise returns `GeneratedCertificate` instance. + """ + try: + # Attempt to load the user's generated certificate data + if preview_mode: + user_certificate = GeneratedCertificate.objects.get( + user=user, + course_id=course_key, + mode=preview_mode + ) + else: + user_certificate = GeneratedCertificate.objects.get( + user=user, + course_id=course_key + ) + + # If there's no generated certificate data for this user, we need to see if we're in 'preview' mode... + # If we are, we'll need to create a mock version of the user_certificate container for previewing + except GeneratedCertificate.DoesNotExist: + if preview_mode and ( + has_access(request.user, 'instructor', course) or + has_access(request.user, 'staff', course)): + user_certificate = GeneratedCertificate( + mode=preview_mode, + verify_uuid=unicode(uuid4().hex), + modified_date=datetime.now().date() + ) + else: + return None + + return user_certificate + + +def _track_certificate_events(request, context, course, user, user_certificate): + """ + Tracks web certificate view related events. + """ + badge = context['badge'] + # Badge Request Event Tracking Logic + if 'evidence_visit' in request.GET and badge: + tracker.emit( + 'edx.badge.assertion.evidence_visited', + { + 'user_id': user.id, + 'course_id': unicode(course.id), + 'enrollment_mode': badge.mode, + 'assertion_id': badge.id, + 'assertion_image_url': badge.data['image'], + 'assertion_json_url': badge.data['json']['id'], + 'issuer': badge.data['issuer'], + } + ) # track certificate evidence_visited event for analytics when certificate_user and accessing_user are different if request.user and request.user.id != user.id: - emit_certificate_event('evidence_visited', user, course_id, course, { + emit_certificate_event('evidence_visited', user, unicode(course.id), course, { 'certificate_id': user_certificate.verify_uuid, 'enrollment_mode': user_certificate.mode, 'social_network': CertificateSocialNetworks.linkedin }) - # Append/Override the existing view context values with any course-specific static values from Advanced Settings - context.update(course.cert_html_view_overrides) - # FINALLY, generate and send the output the client +def _render_certificate_template(request, context, course, user_certificate): + """ + Picks appropriate certificate templates and renders it. + """ if settings.FEATURES.get('CUSTOM_CERTIFICATE_TEMPLATES_ENABLED', False): - custom_template = get_certificate_template(course_key, user_certificate.mode) + custom_template = get_certificate_template(course.id, user_certificate.mode) if custom_template: template = Template( custom_template, @@ -491,3 +418,134 @@ def render_html_view(request, user_id, course_id): return HttpResponse(template.render(context)) return render_to_response("certificates/valid.html", context) + + +def _update_microsite_context(context, configuration): + """ + Updates context with microsites data. + Microsites will need to be able to override any hard coded + content that was put into the context in the + _update_certificate_context() call above. For example the + 'company_about_description' talks about edX, which we most likely + do not want to keep in a microsite + + So we need to re-apply any configuration/content that + we are sourcing from the database. This is somewhat duplicative of + the code at the beginning of this method, but we + need the configuration at the top as some error code paths + require that to be set up early on in the pipeline + """ + + microsite_config_key = microsite.get_value('domain_prefix') + microsites_config = configuration.get("microsites", {}) + if microsite_config_key and microsites_config: + context.update(microsites_config.get(microsite_config_key, {})) + + +def _update_badge_context(context, course, user): + """ + Updates context with badge info. + """ + try: + badge = BadgeAssertion.objects.get(user=user, course_id=course.location.course_key) + except BadgeAssertion.DoesNotExist: + badge = None + context['badge'] = badge + + +def _update_organization_context(context, course): + """ + Updates context with organization related info. + """ + partner_long_name, organization_logo = None, None + partner_short_name = course.display_organization if course.display_organization else course.org + organizations = organization_api.get_course_organizations(course_id=course.id) + if organizations: + #TODO Need to add support for multiple organizations, Currently we are interested in the first one. + organization = organizations[0] + partner_long_name = organization.get('name', partner_long_name) + partner_short_name = organization.get('short_name', partner_short_name) + organization_logo = organization.get('logo', None) + + context['organization_long_name'] = partner_long_name + context['organization_short_name'] = partner_short_name + context['accomplishment_copy_course_org'] = partner_short_name + context['organization_logo'] = organization_logo + + +@handle_500( + template_path="certificates/server-error.html", + test_func=lambda request: request.GET.get('preview', None) +) +def render_html_view(request, user_id, course_id): + """ + This public view generates an HTML representation of the specified student's certificate + If a certificate is not available, we display a "Sorry!" screen instead + """ + preview_mode = request.GET.get('preview', None) + platform_name = microsite.get_value("platform_name", settings.PLATFORM_NAME) + configuration = CertificateHtmlViewConfiguration.get_config() + # Create the initial view context, bootstrapping with Django settings and passed-in values + context = {} + _update_context_with_basic_info(context, course_id, platform_name, configuration) + invalid_template_path = 'certificates/invalid.html' + + # Kick the user back to the "Invalid" screen if the feature is disabled + if not has_html_certificates_enabled(course_id): + return render_to_response(invalid_template_path, context) + + # Load the course and user objects + try: + course_key = CourseKey.from_string(course_id) + user = User.objects.get(id=user_id) + course = modulestore().get_course(course_key) + + # For any other expected exceptions, kick the user back to the "Invalid" screen + except (InvalidKeyError, ItemNotFoundError, User.DoesNotExist): + return render_to_response(invalid_template_path, context) + + # Load user's certificate + user_certificate = _get_user_certificate(request, user, course_key, course, preview_mode) + if not user_certificate: + return render_to_response(invalid_template_path, context) + + # Get the active certificate configuration for this course + # If we do not have an active certificate, we'll need to send the user to the "Invalid" screen + # Passing in the 'preview' parameter, if specified, will return a configuration, if defined + active_configuration = get_active_web_certificate(course, preview_mode) + if active_configuration is None: + return render_to_response(invalid_template_path, context) + context['certificate_data'] = active_configuration + + # Append/Override the existing view context values with any mode-specific ConfigurationModel values + context.update(configuration.get(user_certificate.mode, {})) + + # Append organization info + _update_organization_context(context, course) + + # Append course info + _update_course_context(request, context, course, platform_name) + + # Append user info + _update_context_with_user_info(context, user, user_certificate) + + # Append social sharing info + _update_social_context(request, context, course, user, user_certificate, platform_name) + + # Append/Override the existing view context values with certificate specific values + _update_certificate_context(context, user_certificate, platform_name) + + # Append badge info + _update_badge_context(context, course, user) + + # Append microsite overrides + _update_microsite_context(context, configuration) + + # Append/Override the existing view context values with any course-specific static values from Advanced Settings + context.update(course.cert_html_view_overrides) + + # Track certificate view events + _track_certificate_events(request, context, course, user, user_certificate) + + # FINALLY, render appropriate certificate + return _render_certificate_template(request, context, course, user_certificate) From 88ae4ce2577c4b66264d97d0e74e6e5227f3b267 Mon Sep 17 00:00:00 2001 From: asadiqbal Date: Mon, 16 Nov 2015 13:09:51 +0500 Subject: [PATCH 02/10] SOL-1416 --- cms/static/js/models/settings/course_details.js | 3 --- cms/static/js/spec/views/settings/main_spec.js | 7 ------- 2 files changed, 10 deletions(-) diff --git a/cms/static/js/models/settings/course_details.js b/cms/static/js/models/settings/course_details.js index 9703977314..99db4d68a1 100644 --- a/cms/static/js/models/settings/course_details.js +++ b/cms/static/js/models/settings/course_details.js @@ -35,9 +35,6 @@ var CourseDetails = Backbone.Model.extend({ if (newattrs.start_date === null) { errors.start_date = gettext("The course must have an assigned start date."); } - if (this.hasChanged("start_date") && this.get("has_cert_config") === false){ - errors.start_date = gettext("The course must have at least one active certificate configuration before it can be started."); - } if (newattrs.start_date && newattrs.end_date && newattrs.start_date >= newattrs.end_date) { errors.end_date = gettext("The course end date must be later than the course start date."); } diff --git a/cms/static/js/spec/views/settings/main_spec.js b/cms/static/js/spec/views/settings/main_spec.js index 636ed93c0a..e8284620f8 100644 --- a/cms/static/js/spec/views/settings/main_spec.js +++ b/cms/static/js/spec/views/settings/main_spec.js @@ -72,13 +72,6 @@ define([ ); }); - it('Changing course start date without active certificate configuration should result in error', function () { - this.view.$el.find('#course-start-date') - .val('10/06/2014') - .trigger('change'); - expect(this.view.$el.find('span.message-error').text()).toContain("course must have at least one active certificate configuration"); - }); - it('Selecting a course in pre-requisite drop down should save it as part of course details', function () { var pre_requisite_courses = ['test/CSS101/2012_T1']; var requests = AjaxHelpers.requests(this), From dfa0fe2db7e5cec882cbec5e28aa464cab1333f1 Mon Sep 17 00:00:00 2001 From: raeeschachar Date: Wed, 18 Nov 2015 17:24:22 +0500 Subject: [PATCH 03/10] Fixed Chrome Bok Choy tests --- .../acceptance/tests/studio/test_studio_container.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/common/test/acceptance/tests/studio/test_studio_container.py b/common/test/acceptance/tests/studio/test_studio_container.py index d8cde09d77..040e9ef84d 100644 --- a/common/test/acceptance/tests/studio/test_studio_container.py +++ b/common/test/acceptance/tests/studio/test_studio_container.py @@ -397,6 +397,7 @@ class EditVisibilityModalTest(ContainerBase): # Re-open the modal and inspect its selected inputs visibility_editor = self.edit_component_visibility(component) self.verify_selected_labels(visibility_editor, expected_labels) + visibility_editor.save() def verify_component_validation_error(self, component): """ @@ -427,14 +428,13 @@ class EditVisibilityModalTest(ContainerBase): self.browser.refresh() self.container_page.wait_for_page() - def remove_missing_groups(self, component): + def remove_missing_groups(self, visibility_editor, component): """ Deselect the missing groups for a component. After save, verify that there are no missing group messages in the modal and that there is no validation error on the component. """ - visibility_editor = self.edit_component_visibility(component) - for option in self.edit_component_visibility(component).selected_options: + for option in visibility_editor.selected_options: if option.text == self.MISSING_GROUP_LABEL: option.click() visibility_editor.save() @@ -541,7 +541,7 @@ class EditVisibilityModalTest(ContainerBase): self.verify_component_validation_error(self.html_component) visibility_editor = self.edit_component_visibility(self.html_component) self.verify_selected_labels(visibility_editor, [self.MISSING_GROUP_LABEL] * 2) - self.remove_missing_groups(self.html_component) + self.remove_missing_groups(visibility_editor, self.html_component) self.verify_visibility_set(self.html_component, False) def test_found_and_missing_groups(self): @@ -565,7 +565,7 @@ class EditVisibilityModalTest(ContainerBase): self.verify_component_validation_error(self.html_component) visibility_editor = self.edit_component_visibility(self.html_component) self.verify_selected_labels(visibility_editor, ['Dogs', 'Cats'] + [self.MISSING_GROUP_LABEL] * 2) - self.remove_missing_groups(self.html_component) + self.remove_missing_groups(visibility_editor, self.html_component) visibility_editor = self.edit_component_visibility(self.html_component) self.verify_selected_labels(visibility_editor, ['Dogs', 'Cats']) self.verify_visibility_set(self.html_component, True) From cf5d276aea7ce3dd6805196f35b2af2f3014ddea Mon Sep 17 00:00:00 2001 From: Sven Marnach Date: Mon, 24 Aug 2015 17:31:53 +0200 Subject: [PATCH 04/10] Emit events when users vote on forum posts. --- lms/djangoapps/discussion_api/api.py | 24 +- .../django_comment_client/base/views.py | 243 +++++++++--------- 2 files changed, 126 insertions(+), 141 deletions(-) diff --git a/lms/djangoapps/discussion_api/api.py b/lms/djangoapps/discussion_api/api.py index 1e0923bdf6..2b3dc81405 100644 --- a/lms/djangoapps/discussion_api/api.py +++ b/lms/djangoapps/discussion_api/api.py @@ -25,13 +25,7 @@ from discussion_api.permissions import ( get_initializable_thread_fields, ) from discussion_api.serializers import CommentSerializer, ThreadSerializer, get_context -from django_comment_client.base.views import ( - THREAD_CREATED_EVENT_NAME, - get_comment_created_event_data, - get_comment_created_event_name, - get_thread_created_event_data, - track_forum_event, -) +from django_comment_client.base.views import track_comment_created_event, track_thread_created_event from django_comment_common.signals import ( thread_created, thread_edited, @@ -566,13 +560,7 @@ def create_thread(request, thread_data): api_thread = serializer.data _do_extra_actions(api_thread, cc_thread, thread_data.keys(), actions_form, context) - track_forum_event( - request, - THREAD_CREATED_EVENT_NAME, - course, - cc_thread, - get_thread_created_event_data(cc_thread, followed=actions_form.cleaned_data["following"]) - ) + track_thread_created_event(request, course, cc_thread, actions_form.cleaned_data["following"]) return api_thread @@ -616,13 +604,7 @@ def create_comment(request, comment_data): api_comment = serializer.data _do_extra_actions(api_comment, cc_comment, comment_data.keys(), actions_form, context) - track_forum_event( - request, - get_comment_created_event_name(cc_comment), - context["course"], - cc_comment, - get_comment_created_event_data(cc_comment, cc_thread["commentable_id"], followed=False) - ) + track_comment_created_event(request, context["course"], cc_comment, cc_thread["commentable_id"], followed=False) return api_comment diff --git a/lms/djangoapps/django_comment_client/base/views.py b/lms/djangoapps/django_comment_client/base/views.py index 9c33cfa450..b84fffc460 100644 --- a/lms/djangoapps/django_comment_client/base/views.py +++ b/lms/djangoapps/django_comment_client/base/views.py @@ -49,10 +49,92 @@ import lms.lib.comment_client as cc log = logging.getLogger(__name__) TRACKING_MAX_FORUM_BODY = 2000 +_EVENT_NAME_TEMPLATE = 'edx.forum.{obj_type}.{action_name}' -THREAD_CREATED_EVENT_NAME = "edx.forum.thread.created" -RESPONSE_CREATED_EVENT_NAME = 'edx.forum.response.created' -COMMENT_CREATED_EVENT_NAME = 'edx.forum.comment.created' + +def track_forum_event(request, event_name, course, obj, data, id_map=None): + """ + Send out an analytics event when a forum event happens. Works for threads, + responses to threads, and comments on those responses. + """ + user = request.user + data['id'] = obj.id + commentable_id = data['commentable_id'] + + team = get_team(commentable_id) + if team is not None: + data.update(team_id=team.team_id) + + if id_map is None: + id_map = get_cached_discussion_id_map(course, [commentable_id], user) + if commentable_id in id_map: + data['category_name'] = id_map[commentable_id]["title"] + data['category_id'] = commentable_id + data['url'] = request.META.get('HTTP_REFERER', '') + data['user_forums_roles'] = [ + role.name for role in user.roles.filter(course_id=course.id) + ] + data['user_course_roles'] = [ + role.role for role in user.courseaccessrole_set.filter(course_id=course.id) + ] + + tracker.emit(event_name, data) + + +def track_created_event(request, event_name, course, obj, data): + if len(obj.body) > TRACKING_MAX_FORUM_BODY: + data['truncated'] = True + else: + data['truncated'] = False + data['body'] = obj.body[:TRACKING_MAX_FORUM_BODY] + track_forum_event(request, event_name, course, obj, data) + + +def track_thread_created_event(request, course, thread, followed): + event_name = _EVENT_NAME_TEMPLATE.format(obj_type='thread', action_name='created') + event_data = { + 'commentable_id': thread.commentable_id, + 'group_id': thread.get("group_id"), + 'thread_type': thread.thread_type, + 'title': thread.title, + 'anonymous': thread.anonymous, + 'anonymous_to_peers': thread.anonymous_to_peers, + 'options': {'followed': followed}, + # There is a stated desire for an 'origin' property that will state + # whether this thread was created via courseware or the forum. + # However, the view does not contain that data, and including it will + # likely require changes elsewhere. + } + track_created_event(request, event_name, course, thread, event_data) + + +def track_comment_created_event(request, course, comment, commentable_id, followed): + obj_type = 'comment' if comment.get("parent_id") else 'response' + event_name = _EVENT_NAME_TEMPLATE.format(obj_type=obj_type, action_name='created') + event_data = { + 'discussion': {'id': comment.thread_id}, + 'commentable_id': commentable_id, + 'options': {'followed': followed}, + } + parent_id = comment.get('parent_id') + if parent_id: + event_data['response'] = {'id': parent_id} + track_created_event(request, event_name, course, comment, event_data) + + +def track_voted_event(request, course, obj, vote_value, undo_vote=False): + if isinstance(obj, cc.Thread): + obj_type = 'thread' + else: + obj_type = 'response' + event_name = _EVENT_NAME_TEMPLATE.format(obj_type=obj_type, action_name='voted') + event_data = { + 'commentable_id': obj.commentable_id, + 'target_username': obj.get('username'), + 'undo_vote': undo_vote, + 'vote_value': vote_value, + } + track_forum_event(request, event_name, course, obj, event_data) def permitted(fn): @@ -85,85 +167,6 @@ def ajax_content_response(request, course_key, content): }) -def track_forum_event(request, event_name, course, obj, data, id_map=None): - """ - Send out an analytics event when a forum event happens. Works for threads, - responses to threads, and comments on those responses. - """ - user = request.user - data['id'] = obj.id - commentable_id = data['commentable_id'] - - team = get_team(commentable_id) - if team is not None: - data.update(team_id=team.team_id) - - if id_map is None: - id_map = get_cached_discussion_id_map(course, [commentable_id], user) - - if commentable_id in id_map: - data['category_name'] = id_map[commentable_id]["title"] - data['category_id'] = commentable_id - if len(obj.body) > TRACKING_MAX_FORUM_BODY: - data['truncated'] = True - else: - data['truncated'] = False - - data['body'] = obj.body[:TRACKING_MAX_FORUM_BODY] - data['url'] = request.META.get('HTTP_REFERER', '') - data['user_forums_roles'] = [ - role.name for role in user.roles.filter(course_id=course.id) - ] - data['user_course_roles'] = [ - role.role for role in user.courseaccessrole_set.filter(course_id=course.id) - ] - - tracker.emit(event_name, data) - - -def get_thread_created_event_data(thread, followed): - """ - Get the event data payload for thread creation (excluding fields populated - by track_forum_event) - """ - return { - 'commentable_id': thread.commentable_id, - 'group_id': thread.get("group_id"), - 'thread_type': thread.thread_type, - 'title': thread.title, - 'anonymous': thread.anonymous, - 'anonymous_to_peers': thread.anonymous_to_peers, - 'options': {'followed': followed}, - # There is a stated desire for an 'origin' property that will state - # whether this thread was created via courseware or the forum. - # However, the view does not contain that data, and including it will - # likely require changes elsewhere. - } - - -def get_comment_created_event_name(comment): - """Get the appropriate event name for creating a response/comment""" - return COMMENT_CREATED_EVENT_NAME if comment.get("parent_id") else RESPONSE_CREATED_EVENT_NAME - - -def get_comment_created_event_data(comment, commentable_id, followed): - """ - Get the event data payload for comment creation (excluding fields populated - by track_forum_event) - """ - event_data = { - 'discussion': {'id': comment.thread_id}, - 'commentable_id': commentable_id, - 'options': {'followed': followed}, - } - - parent_id = comment.get("parent_id") - if parent_id: - event_data['response'] = {'id': parent_id} - - return event_data - - @require_POST @login_required @permitted @@ -234,12 +237,11 @@ def create_thread(request, course_id, commentable_id): cc_user = cc.User.from_django_user(user) cc_user.follow(thread) - event_data = get_thread_created_event_data(thread, follow) data = thread.to_dict() add_courseware_context([data], course, user) - track_forum_event(request, THREAD_CREATED_EVENT_NAME, course, thread, event_data) + track_thread_created_event(request, course, thread, follow) if request.is_ajax(): return ajax_content_response(request, course_key, data) @@ -330,9 +332,7 @@ def _create_comment(request, course_key, thread_id=None, parent_id=None): cc_user = cc.User.from_django_user(request.user) cc_user.follow(comment.thread) - event_name = get_comment_created_event_name(comment) - event_data = get_comment_created_event_data(comment, comment.thread.commentable_id, followed) - track_forum_event(request, event_name, course, comment, event_data) + track_comment_created_event(request, course, comment, comment.thread.commentable_id, followed) if request.is_ajax(): return ajax_content_response(request, course_key, comment.to_dict()) @@ -456,6 +456,24 @@ def delete_comment(request, course_id, comment_id): return JsonResponse(prepare_content(comment.to_dict(), course_key)) +def _vote_or_unvote(request, course_id, obj, value='up', undo_vote=False): + """ + Vote or unvote for a thread or a response. + """ + course_key = SlashSeparatedCourseKey.from_deprecated_string(course_id) + course = get_course_with_access(request.user, 'load', course_key) + user = cc.User.from_django_user(request.user) + if undo_vote: + user.unvote(obj) + # TODO(smarnach): Determine the value of the vote that is undone. Currently, you can + # only cast upvotes in the user interface, so it is assumed that the vote value is 'up'. + # (People could theoretically downvote by handcrafting AJAX requests.) + else: + user.vote(obj, value) + track_voted_event(request, course, obj, value, undo_vote) + return JsonResponse(prepare_content(obj.to_dict(), course_key)) + + @require_POST @login_required @permitted @@ -463,13 +481,10 @@ def vote_for_comment(request, course_id, comment_id, value): """ given a course_id and comment_id, """ - course_key = SlashSeparatedCourseKey.from_deprecated_string(course_id) - user = request.user - cc_user = cc.User.from_django_user(user) comment = cc.Comment.find(comment_id) - cc_user.vote(comment, value) - comment_voted.send(sender=None, user=user, post=comment) - return JsonResponse(prepare_content(comment.to_dict(), course_key)) + result = _vote_or_unvote(request, course_id, comment, value) + comment_voted.send(sender=None, user=request.user, post=comment) + return result @require_POST @@ -480,11 +495,7 @@ def undo_vote_for_comment(request, course_id, comment_id): given a course id and comment id, remove vote ajax only """ - course_key = SlashSeparatedCourseKey.from_deprecated_string(course_id) - user = cc.User.from_django_user(request.user) - comment = cc.Comment.find(comment_id) - user.unvote(comment) - return JsonResponse(prepare_content(comment.to_dict(), course_key)) + return _vote_or_unvote(request, course_id, cc.Comment.find(comment_id), undo_vote=True) @require_POST @@ -495,13 +506,21 @@ def vote_for_thread(request, course_id, thread_id, value): given a course id and thread id vote for this thread ajax only """ - course_key = SlashSeparatedCourseKey.from_deprecated_string(course_id) - user = request.user - cc_user = cc.User.from_django_user(user) thread = cc.Thread.find(thread_id) - cc_user.vote(thread, value) - thread_voted.send(sender=None, user=user, post=thread) - return JsonResponse(prepare_content(thread.to_dict(), course_key)) + result = _vote_or_unvote(request, course_id, thread, value) + thread_voted.send(sender=None, user=request.user, post=thread) + return result + + +@require_POST +@login_required +@permitted +def undo_vote_for_thread(request, course_id, thread_id): + """ + given a course id and thread id, remove users vote for thread + ajax only + """ + return _vote_or_unvote(request, course_id, cc.Thread.find(thread_id), undo_vote=True) @require_POST @@ -576,22 +595,6 @@ def un_flag_abuse_for_comment(request, course_id, comment_id): return JsonResponse(prepare_content(comment.to_dict(), course_key)) -@require_POST -@login_required -@permitted -def undo_vote_for_thread(request, course_id, thread_id): - """ - given a course id and thread id, remove users vote for thread - ajax only - """ - course_key = SlashSeparatedCourseKey.from_deprecated_string(course_id) - user = cc.User.from_django_user(request.user) - thread = cc.Thread.find(thread_id) - user.unvote(thread) - - return JsonResponse(prepare_content(thread.to_dict(), course_key)) - - @require_POST @login_required @permitted From ef563e428574b8cdb633372eb3ef0ea793dcbaa7 Mon Sep 17 00:00:00 2001 From: Sven Marnach Date: Sat, 5 Sep 2015 23:24:07 +0200 Subject: [PATCH 05/10] Add tests for the discussion forum vote events. --- .../django_comment_client/base/tests.py | 34 +++++++++++++++++++ .../django_comment_client/base/views.py | 32 ++++++++++++++--- 2 files changed, 62 insertions(+), 4 deletions(-) diff --git a/lms/djangoapps/django_comment_client/base/tests.py b/lms/djangoapps/django_comment_client/base/tests.py index 3e0dfd8129..9f14f2daa4 100644 --- a/lms/djangoapps/django_comment_client/base/tests.py +++ b/lms/djangoapps/django_comment_client/base/tests.py @@ -1641,6 +1641,40 @@ class ForumEventTestCase(ModuleStoreTestCase, MockRequestSetupMixin): self.assertEqual(name, event_name) self.assertEqual(event['team_id'], team.team_id) + @ddt.data( + ('vote_for_thread', 'thread_id', 'thread'), + ('undo_vote_for_thread', 'thread_id', 'thread'), + ('vote_for_comment', 'comment_id', 'response'), + ('undo_vote_for_comment', 'comment_id', 'response'), + ) + @ddt.unpack + @patch('eventtracking.tracker.emit') + @patch('lms.lib.comment_client.utils.requests.request') + def test_thread_voted_event(self, view_name, obj_id_name, obj_type, mock_request, mock_emit): + undo = view_name.startswith('undo') + + self._set_mock_request_data(mock_request, { + 'closed': False, + 'commentable_id': 'test_commentable_id', + 'username': 'gumprecht', + }) + request = RequestFactory().post('dummy_url', {}) + request.user = self.student + request.view_name = view_name + view_function = getattr(views, view_name) + kwargs = dict(course_id=unicode(self.course.id)) + kwargs[obj_id_name] = obj_id_name + if not undo: + kwargs.update(value='up') + view_function(request, **kwargs) + + self.assertTrue(mock_emit.called) + event_name, event = mock_emit.call_args[0] + self.assertEqual(event_name, 'edx.forum.{}.voted'.format(obj_type)) + self.assertEqual(event['target_username'], 'gumprecht') + self.assertEqual(event['undo_vote'], undo) + self.assertEqual(event['vote_value'], 'up') + class UsersEndpointTestCase(ModuleStoreTestCase, MockRequestSetupMixin): diff --git a/lms/djangoapps/django_comment_client/base/views.py b/lms/djangoapps/django_comment_client/base/views.py index b84fffc460..e480ff62a3 100644 --- a/lms/djangoapps/django_comment_client/base/views.py +++ b/lms/djangoapps/django_comment_client/base/views.py @@ -82,6 +82,9 @@ def track_forum_event(request, event_name, course, obj, data, id_map=None): def track_created_event(request, event_name, course, obj, data): + """ + Send analytics event for a newly created thread, response or comment. + """ if len(obj.body) > TRACKING_MAX_FORUM_BODY: data['truncated'] = True else: @@ -91,6 +94,9 @@ def track_created_event(request, event_name, course, obj, data): def track_thread_created_event(request, course, thread, followed): + """ + Send analytics event for a newly created thread. + """ event_name = _EVENT_NAME_TEMPLATE.format(obj_type='thread', action_name='created') event_data = { 'commentable_id': thread.commentable_id, @@ -109,6 +115,9 @@ def track_thread_created_event(request, course, thread, followed): def track_comment_created_event(request, course, comment, commentable_id, followed): + """ + Send analytics event for a newly created response or comment. + """ obj_type = 'comment' if comment.get("parent_id") else 'response' event_name = _EVENT_NAME_TEMPLATE.format(obj_type=obj_type, action_name='created') event_data = { @@ -123,6 +132,9 @@ def track_comment_created_event(request, course, comment, commentable_id, follow def track_voted_event(request, course, obj, vote_value, undo_vote=False): + """ + Send analytics event for a vote on a thread or response. + """ if isinstance(obj, cc.Thread): obj_type = 'thread' else: @@ -137,10 +149,19 @@ def track_voted_event(request, course, obj, vote_value, undo_vote=False): track_forum_event(request, event_name, course, obj, event_data) -def permitted(fn): - @functools.wraps(fn) +def permitted(func): + """ + View decorator to verify the user is authorized to access this endpoint. + """ + @functools.wraps(func) def wrapper(request, *args, **kwargs): + """ + Wrapper for the view that only calls the view if the user is authorized. + """ def fetch_content(): + """ + Extract the forum object from the keyword arguments to the view. + """ if "thread_id" in kwargs: content = cc.Thread.find(kwargs["thread_id"]).to_dict() elif "comment_id" in kwargs: @@ -152,13 +173,16 @@ def permitted(fn): return content course_key = SlashSeparatedCourseKey.from_deprecated_string(kwargs['course_id']) if check_permissions_by_view(request.user, course_key, fetch_content(), request.view_name): - return fn(request, *args, **kwargs) + return func(request, *args, **kwargs) else: return JsonError("unauthorized", status=401) return wrapper def ajax_content_response(request, course_key, content): + """ + Standard AJAX response returning the content hierarchy of the current thread. + """ user_info = cc.User.from_django_user(request.user).to_dict() annotated_content_info = get_annotated_content_info(course_key, content, request.user, user_info) return JsonResponse({ @@ -479,7 +503,7 @@ def _vote_or_unvote(request, course_id, obj, value='up', undo_vote=False): @permitted def vote_for_comment(request, course_id, comment_id, value): """ - given a course_id and comment_id, + Given a course_id and comment_id, vote for this response. AJAX only. """ comment = cc.Comment.find(comment_id) result = _vote_or_unvote(request, course_id, comment, value) From 3dcc3c4aae28dc3f7506eab0b4db0bf854d5c1b2 Mon Sep 17 00:00:00 2001 From: Sven Marnach Date: Wed, 18 Nov 2015 14:17:31 +0100 Subject: [PATCH 06/10] Clean up occurrences of SlashSeparatedCourseKey in the django_comment_client app. The preferred form is to use CourseKey.from_string() instead of SlashSeparatedCourseKey.from_deprecated_string(). --- .../django_comment_client/base/tests.py | 4 +- .../django_comment_client/base/views.py | 37 +++++++++---------- .../commands/get_discussion_link.py | 6 +-- .../commands/seed_permissions_roles.py | 4 +- .../tests/test_models.py | 6 +-- 5 files changed, 26 insertions(+), 31 deletions(-) diff --git a/lms/djangoapps/django_comment_client/base/tests.py b/lms/djangoapps/django_comment_client/base/tests.py index 9f14f2daa4..f8fefdbf23 100644 --- a/lms/djangoapps/django_comment_client/base/tests.py +++ b/lms/djangoapps/django_comment_client/base/tests.py @@ -13,7 +13,7 @@ from django.core.urlresolvers import reverse from request_cache.middleware import RequestCache from mock import patch, ANY, Mock from nose.tools import assert_true, assert_equal # pylint: disable=no-name-in-module -from opaque_keys.edx.locations import SlashSeparatedCourseKey +from opaque_keys.edx.keys import CourseKey from lms.lib.comment_client import Thread from common.test.utils import MockSignalHandlerMixin, disable_signal @@ -1733,7 +1733,7 @@ class UsersEndpointTestCase(ModuleStoreTestCase, MockRequestSetupMixin): self.assertNotIn("users", content) def test_course_does_not_exist(self): - course_id = SlashSeparatedCourseKey.from_deprecated_string("does/not/exist") + course_id = CourseKey.from_string("does/not/exist") response = self.make_request(course_id=course_id, username="other") self.assertEqual(response.status_code, 404) diff --git a/lms/djangoapps/django_comment_client/base/views.py b/lms/djangoapps/django_comment_client/base/views.py index e480ff62a3..d60860f591 100644 --- a/lms/djangoapps/django_comment_client/base/views.py +++ b/lms/djangoapps/django_comment_client/base/views.py @@ -12,7 +12,6 @@ from django.utils.translation import ugettext as _ from django.views.decorators import csrf from django.views.decorators.http import require_GET, require_POST from opaque_keys.edx.keys import CourseKey -from opaque_keys.edx.locations import SlashSeparatedCourseKey from courseware.access import has_access from util.file import store_uploaded_file @@ -171,7 +170,7 @@ def permitted(func): else: content = None return content - course_key = SlashSeparatedCourseKey.from_deprecated_string(kwargs['course_id']) + course_key = CourseKey.from_string(kwargs['course_id']) if check_permissions_by_view(request.user, course_key, fetch_content(), request.view_name): return func(request, *args, **kwargs) else: @@ -200,7 +199,7 @@ def create_thread(request, course_id, commentable_id): """ log.debug("Creating new thread in %r, id %r", course_id, commentable_id) - course_key = SlashSeparatedCourseKey.from_deprecated_string(course_id) + course_key = CourseKey.from_string(course_id) course = get_course_with_access(request.user, 'load', course_key) post = request.POST user = request.user @@ -285,7 +284,7 @@ def update_thread(request, course_id, thread_id): if 'body' not in request.POST or not request.POST['body'].strip(): return JsonError(_("Body can't be empty")) - course_key = SlashSeparatedCourseKey.from_deprecated_string(course_id) + course_key = CourseKey.from_string(course_id) thread = cc.Thread.find(thread_id) # Get thread context first in order to be safe from reseting the values of thread object later thread_context = getattr(thread, "context", "course") @@ -374,7 +373,7 @@ def create_comment(request, course_id, thread_id): """ if is_comment_too_deep(parent=None): return JsonError(_("Comment level too deep")) - return _create_comment(request, SlashSeparatedCourseKey.from_deprecated_string(course_id), thread_id=thread_id) + return _create_comment(request, CourseKey.from_string(course_id), thread_id=thread_id) @require_POST @@ -385,7 +384,7 @@ def delete_thread(request, course_id, thread_id): # pylint: disable=unused-argu given a course_id and thread_id, delete this thread this is ajax only """ - course_key = SlashSeparatedCourseKey.from_deprecated_string(course_id) + course_key = CourseKey.from_string(course_id) thread = cc.Thread.find(thread_id) thread.delete() thread_deleted.send(sender=None, user=request.user, post=thread) @@ -400,7 +399,7 @@ def update_comment(request, course_id, comment_id): given a course_id and comment_id, update the comment with payload attributes handles static and ajax submissions """ - course_key = SlashSeparatedCourseKey.from_deprecated_string(course_id) + course_key = CourseKey.from_string(course_id) comment = cc.Comment.find(comment_id) if 'body' not in request.POST or not request.POST['body'].strip(): return JsonError(_("Body can't be empty")) @@ -423,7 +422,7 @@ def endorse_comment(request, course_id, comment_id): given a course_id and comment_id, toggle the endorsement of this comment, ajax only """ - course_key = SlashSeparatedCourseKey.from_deprecated_string(course_id) + course_key = CourseKey.from_string(course_id) comment = cc.Comment.find(comment_id) user = request.user comment.endorsed = request.POST.get('endorsed', 'false').lower() == 'true' @@ -441,7 +440,7 @@ def openclose_thread(request, course_id, thread_id): given a course_id and thread_id, toggle the status of this thread ajax only """ - course_key = SlashSeparatedCourseKey.from_deprecated_string(course_id) + course_key = CourseKey.from_string(course_id) thread = cc.Thread.find(thread_id) thread.closed = request.POST.get('closed', 'false').lower() == 'true' thread.save() @@ -462,7 +461,7 @@ def create_sub_comment(request, course_id, comment_id): """ if is_comment_too_deep(parent=cc.Comment(comment_id)): return JsonError(_("Comment level too deep")) - return _create_comment(request, SlashSeparatedCourseKey.from_deprecated_string(course_id), parent_id=comment_id) + return _create_comment(request, CourseKey.from_string(course_id), parent_id=comment_id) @require_POST @@ -473,7 +472,7 @@ def delete_comment(request, course_id, comment_id): given a course_id and comment_id delete this comment ajax only """ - course_key = SlashSeparatedCourseKey.from_deprecated_string(course_id) + course_key = CourseKey.from_string(course_id) comment = cc.Comment.find(comment_id) comment.delete() comment_deleted.send(sender=None, user=request.user, post=comment) @@ -484,7 +483,7 @@ def _vote_or_unvote(request, course_id, obj, value='up', undo_vote=False): """ Vote or unvote for a thread or a response. """ - course_key = SlashSeparatedCourseKey.from_deprecated_string(course_id) + course_key = CourseKey.from_string(course_id) course = get_course_with_access(request.user, 'load', course_key) user = cc.User.from_django_user(request.user) if undo_vote: @@ -555,7 +554,7 @@ def flag_abuse_for_thread(request, course_id, thread_id): given a course_id and thread_id flag this thread for abuse ajax only """ - course_key = SlashSeparatedCourseKey.from_deprecated_string(course_id) + course_key = CourseKey.from_string(course_id) user = cc.User.from_django_user(request.user) thread = cc.Thread.find(thread_id) thread.flagAbuse(user, thread) @@ -572,7 +571,7 @@ def un_flag_abuse_for_thread(request, course_id, thread_id): ajax only """ user = cc.User.from_django_user(request.user) - course_key = SlashSeparatedCourseKey.from_deprecated_string(course_id) + course_key = CourseKey.from_string(course_id) course = get_course_by_id(course_key) thread = cc.Thread.find(thread_id) remove_all = bool( @@ -592,7 +591,7 @@ def flag_abuse_for_comment(request, course_id, comment_id): given a course and comment id, flag comment for abuse ajax only """ - course_key = SlashSeparatedCourseKey.from_deprecated_string(course_id) + course_key = CourseKey.from_string(course_id) user = cc.User.from_django_user(request.user) comment = cc.Comment.find(comment_id) comment.flagAbuse(user, comment) @@ -608,7 +607,7 @@ def un_flag_abuse_for_comment(request, course_id, comment_id): ajax only """ user = cc.User.from_django_user(request.user) - course_key = SlashSeparatedCourseKey.from_deprecated_string(course_id) + course_key = CourseKey.from_string(course_id) course = get_course_by_id(course_key) remove_all = bool( has_permission(request.user, 'openclose_thread', course_key) or @@ -627,7 +626,7 @@ def pin_thread(request, course_id, thread_id): given a course id and thread id, pin this thread ajax only """ - course_key = SlashSeparatedCourseKey.from_deprecated_string(course_id) + course_key = CourseKey.from_string(course_id) user = cc.User.from_django_user(request.user) thread = cc.Thread.find(thread_id) thread.pin(user, thread_id) @@ -643,7 +642,7 @@ def un_pin_thread(request, course_id, thread_id): given a course id and thread id, remove pin from this thread ajax only """ - course_key = SlashSeparatedCourseKey.from_deprecated_string(course_id) + course_key = CourseKey.from_string(course_id) user = cc.User.from_django_user(request.user) thread = cc.Thread.find(thread_id) thread.un_pin(user, thread_id) @@ -769,7 +768,7 @@ def users(request, course_id): Only exact matches are supported here, so the length of the result set will either be 0 or 1. """ - course_key = SlashSeparatedCourseKey.from_deprecated_string(course_id) + course_key = CourseKey.from_string(course_id) try: get_course_with_access(request.user, 'load', course_key, check_if_enrolled=True) except Http404: diff --git a/lms/djangoapps/django_comment_client/management/commands/get_discussion_link.py b/lms/djangoapps/django_comment_client/management/commands/get_discussion_link.py index 80419db8f6..603f0269f9 100644 --- a/lms/djangoapps/django_comment_client/management/commands/get_discussion_link.py +++ b/lms/djangoapps/django_comment_client/management/commands/get_discussion_link.py @@ -1,7 +1,6 @@ from django.core.management.base import BaseCommand, CommandError from opaque_keys import InvalidKeyError from opaque_keys.edx.keys import CourseKey -from opaque_keys.edx.locations import SlashSeparatedCourseKey from courseware.courses import get_course @@ -16,10 +15,7 @@ class Command(BaseCommand): raise CommandError("Only one course id may be specifiied") course_id = args[0] - try: - course_key = CourseKey.from_string(course_id) - except InvalidKeyError: - course_key = SlashSeparatedCourseKey.from_deprecated_string(course_id) + course_key = CourseKey.from_string(course_id) course = get_course(course_key) if not course: diff --git a/lms/djangoapps/django_comment_client/management/commands/seed_permissions_roles.py b/lms/djangoapps/django_comment_client/management/commands/seed_permissions_roles.py index 6e4fabb6b6..64976656b2 100644 --- a/lms/djangoapps/django_comment_client/management/commands/seed_permissions_roles.py +++ b/lms/djangoapps/django_comment_client/management/commands/seed_permissions_roles.py @@ -3,7 +3,7 @@ Management command to seed default permissions and roles. """ from django.core.management.base import BaseCommand, CommandError from django_comment_common.utils import seed_permissions_roles -from opaque_keys.edx.locations import SlashSeparatedCourseKey +from opaque_keys.edx.keys import CourseKey class Command(BaseCommand): @@ -15,6 +15,6 @@ class Command(BaseCommand): raise CommandError("Please provide a course id") if len(args) > 1: raise CommandError("Too many arguments") - course_id = SlashSeparatedCourseKey.from_deprecated_string(args[0]) + course_id = CourseKey.from_string(args[0]) seed_permissions_roles(course_id) diff --git a/lms/djangoapps/django_comment_client/tests/test_models.py b/lms/djangoapps/django_comment_client/tests/test_models.py index e243f6d609..4093252990 100644 --- a/lms/djangoapps/django_comment_client/tests/test_models.py +++ b/lms/djangoapps/django_comment_client/tests/test_models.py @@ -3,7 +3,7 @@ Tests for the django comment client integration models """ from django.test.testcases import TestCase from nose.plugins.attrib import attr -from opaque_keys.edx.locations import SlashSeparatedCourseKey +from opaque_keys.edx.keys import CourseKey from xmodule.modulestore.tests.django_utils import TEST_DATA_MIXED_TOY_MODULESTORE import django_comment_common.models as models @@ -23,7 +23,7 @@ class RoleClassTestCase(ModuleStoreTestCase): # For course ID, syntax edx/classname/classdate is important # because xmodel.course_module.id_to_location looks for a string to split - self.course_id = SlashSeparatedCourseKey("edX", "toy", "2012_Fall") + self.course_id = CourseKey.from_string("edX/toy/2012_Fall") self.student_role = models.Role.objects.get_or_create(name="Student", course_id=self.course_id)[0] self.student_role.add_permission("delete_thread") @@ -31,7 +31,7 @@ class RoleClassTestCase(ModuleStoreTestCase): course_id=self.course_id)[0] self.TA_role = models.Role.objects.get_or_create(name="Community TA", course_id=self.course_id)[0] - self.course_id_2 = SlashSeparatedCourseKey("edx", "6.002x", "2012_Fall") + self.course_id_2 = CourseKey.from_string("edX/6.002x/2012_Fall") self.TA_role_2 = models.Role.objects.get_or_create(name="Community TA", course_id=self.course_id_2)[0] From 284dc7a3c652f7d26539fb6e9336fef44cdde776 Mon Sep 17 00:00:00 2001 From: Peter Fogg Date: Wed, 18 Nov 2015 09:06:41 -0500 Subject: [PATCH 07/10] Display an error message if no payment processors are available. Otto is gaining the ability to temporarily disable payment processors. If all are disabled, no checkout buttons will appear on the payment page, and so we need to communicate to the user that they cann't pay right now but should try again later. ECOM-2346 --- .../make_payment_step_view_spec.js | 10 ++++++++++ .../verify_student/pay_and_verify_view_spec.js | 3 ++- .../views/make_payment_step_view.js | 18 ++++++++++++++---- 3 files changed, 26 insertions(+), 5 deletions(-) diff --git a/lms/static/js/spec/verify_student/make_payment_step_view_spec.js b/lms/static/js/spec/verify_student/make_payment_step_view_spec.js index 44d7403313..15184a1e32 100644 --- a/lms/static/js/spec/verify_student/make_payment_step_view_spec.js +++ b/lms/static/js/spec/verify_student/make_payment_step_view_spec.js @@ -206,6 +206,16 @@ define([ expectPaymentButtonEnabled( true ); }); + it('displays an error if no payment processors are available', function () { + var view = createView({processors: []}); + expect(view.errorModel.get('shown')).toBe(true); + expect(view.errorModel.get('errorTitle')).toEqual( + "We're currently experiencing technical problems." + ); + expect(view.errorModel.get('errorMsg')).toEqual( + 'Try the transaction again in a few minutes.' + ); + }); }); } ); diff --git a/lms/static/js/spec/verify_student/pay_and_verify_view_spec.js b/lms/static/js/spec/verify_student/pay_and_verify_view_spec.js index 29fbf52b32..8fb53b910f 100644 --- a/lms/static/js/spec/verify_student/pay_and_verify_view_spec.js +++ b/lms/static/js/spec/verify_student/pay_and_verify_view_spec.js @@ -56,7 +56,8 @@ define(['jquery', 'common/js/spec_helpers/template_helpers', 'js/verify_student/ var createView = function( displaySteps, currentStep ) { return new PayAndVerifyView({ displaySteps: displaySteps, - currentStep: currentStep + currentStep: currentStep, + errorModel: new ( Backbone.Model.extend({}) )() }).render(); }; diff --git a/lms/static/js/verify_student/views/make_payment_step_view.js b/lms/static/js/verify_student/views/make_payment_step_view.js index 9aee49a7b4..4388d0a0da 100644 --- a/lms/static/js/verify_student/views/make_payment_step_view.js +++ b/lms/static/js/verify_student/views/make_payment_step_view.js @@ -105,10 +105,20 @@ var edx = edx || {}; self._getProductText( templateContext.courseModeSlug, templateContext.upgrade ) ); - // create a button for each payment processor - _.each(processors.reverse(), function(processorName) { - $( 'div.payment-buttons' ).append( self._getPaymentButtonHtml(processorName) ); - }); + if (processors.length === 0) { + // No payment processors are enabled at the moment, so show an error message + this.errorModel.set({ + errorTitle: gettext("We're currently experiencing technical problems."), + errorMsg: gettext('Try the transaction again in a few minutes.'), + shown: true + }) + } + else { + // create a button for each payment processor + _.each(processors.reverse(), function(processorName) { + $( 'div.payment-buttons' ).append( self._getPaymentButtonHtml(processorName) ); + }); + } // Handle payment submission $( '.payment-button' ).on( 'click', _.bind( this.createOrder, this ) ); From 60f6bbe92933fdfdacfab80ab07dc58fbdc909bb Mon Sep 17 00:00:00 2001 From: Peter Fogg Date: Wed, 18 Nov 2015 14:05:37 -0500 Subject: [PATCH 08/10] Add correct payment processor messages. (facepalm) --- .../js/spec/verify_student/make_payment_step_view_spec.js | 2 +- lms/static/js/verify_student/views/make_payment_step_view.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/lms/static/js/spec/verify_student/make_payment_step_view_spec.js b/lms/static/js/spec/verify_student/make_payment_step_view_spec.js index 15184a1e32..034c1f3b24 100644 --- a/lms/static/js/spec/verify_student/make_payment_step_view_spec.js +++ b/lms/static/js/spec/verify_student/make_payment_step_view_spec.js @@ -210,7 +210,7 @@ define([ var view = createView({processors: []}); expect(view.errorModel.get('shown')).toBe(true); expect(view.errorModel.get('errorTitle')).toEqual( - "We're currently experiencing technical problems." + 'All payment options are currently unavailable.' ); expect(view.errorModel.get('errorMsg')).toEqual( 'Try the transaction again in a few minutes.' diff --git a/lms/static/js/verify_student/views/make_payment_step_view.js b/lms/static/js/verify_student/views/make_payment_step_view.js index 4388d0a0da..42c0176e58 100644 --- a/lms/static/js/verify_student/views/make_payment_step_view.js +++ b/lms/static/js/verify_student/views/make_payment_step_view.js @@ -108,7 +108,7 @@ var edx = edx || {}; if (processors.length === 0) { // No payment processors are enabled at the moment, so show an error message this.errorModel.set({ - errorTitle: gettext("We're currently experiencing technical problems."), + errorTitle: gettext('All payment options are currently unavailable.'), errorMsg: gettext('Try the transaction again in a few minutes.'), shown: true }) From 0d68cd01bffd1fb599bb87126f2018dbc5d8038a Mon Sep 17 00:00:00 2001 From: Ben Patterson Date: Wed, 18 Nov 2015 12:33:35 -0500 Subject: [PATCH 09/10] Rebalance shard 7. --- .../test/acceptance/tests/lms/test_lms_user_preview.py | 8 ++++++++ .../test/acceptance/tests/studio/test_import_export.py | 9 +++++++++ .../acceptance/tests/studio/test_studio_container.py | 3 +++ .../test/acceptance/tests/studio/test_studio_outline.py | 1 + .../tests/studio/test_studio_settings_details.py | 4 ++++ 5 files changed, 25 insertions(+) diff --git a/common/test/acceptance/tests/lms/test_lms_user_preview.py b/common/test/acceptance/tests/lms/test_lms_user_preview.py index 672be99392..ef0ae96964 100644 --- a/common/test/acceptance/tests/lms/test_lms_user_preview.py +++ b/common/test/acceptance/tests/lms/test_lms_user_preview.py @@ -3,6 +3,9 @@ Tests the "preview" selector in the LMS that allows changing between Staff, Student, and Content Groups. """ + +from nose.plugins.attrib import attr + from ..helpers import UniqueCourseTest, create_user_partition_json from ...pages.studio.auto_auth import AutoAuthPage from ...pages.lms.courseware import CoursewarePage @@ -14,6 +17,7 @@ from xmodule.partitions.partitions import Group from textwrap import dedent +@attr('shard_3') class StaffViewTest(UniqueCourseTest): """ Tests that verify the staff view. @@ -51,6 +55,7 @@ class StaffViewTest(UniqueCourseTest): return staff_page +@attr('shard_3') class CourseWithoutContentGroupsTest(StaffViewTest): """ Setup for tests that have no content restricted to specific content groups. @@ -81,6 +86,7 @@ class CourseWithoutContentGroupsTest(StaffViewTest): ) +@attr('shard_3') class StaffViewToggleTest(CourseWithoutContentGroupsTest): """ Tests for the staff view toggle button. @@ -97,6 +103,7 @@ class StaffViewToggleTest(CourseWithoutContentGroupsTest): self.assertFalse(course_page.has_tab('Instructor')) +@attr('shard_3') class StaffDebugTest(CourseWithoutContentGroupsTest): """ Tests that verify the staff debug info. @@ -228,6 +235,7 @@ class StaffDebugTest(CourseWithoutContentGroupsTest): 'for user {}'.format(self.USERNAME), msg) +@attr('shard_3') class CourseWithContentGroupsTest(StaffViewTest): """ Verifies that changing the "View this course as" selector works properly for content groups. diff --git a/common/test/acceptance/tests/studio/test_import_export.py b/common/test/acceptance/tests/studio/test_import_export.py index ecfb0eaeac..64e0db3980 100644 --- a/common/test/acceptance/tests/studio/test_import_export.py +++ b/common/test/acceptance/tests/studio/test_import_export.py @@ -1,6 +1,7 @@ """ Acceptance tests for the Import and Export pages """ +from nose.plugins.attrib import attr from datetime import datetime from abc import abstractmethod @@ -33,6 +34,7 @@ class ExportTestMixin(object): self.assertTrue(is_tarball_mimetype) +@attr('shard_4') class TestCourseExport(ExportTestMixin, StudioCourseTest): """ Export tests for courses. @@ -55,6 +57,7 @@ class TestCourseExport(ExportTestMixin, StudioCourseTest): self.assertEqual(self.export_page.header_text, 'Course Export') +@attr('shard_4') class TestLibraryExport(ExportTestMixin, StudioLibraryTest): """ Export tests for libraries. @@ -103,6 +106,7 @@ class BadExportMixin(object): ) +@attr('shard_4') class TestLibraryBadExport(BadExportMixin, StudioLibraryTest): """ Verify exporting a bad library causes an error. @@ -126,6 +130,7 @@ class TestLibraryBadExport(BadExportMixin, StudioLibraryTest): ) +@attr('shard_4') class TestCourseBadExport(BadExportMixin, StudioCourseTest): """ Verify exporting a bad course causes an error. @@ -157,6 +162,7 @@ class TestCourseBadExport(BadExportMixin, StudioCourseTest): ) +@attr('shard_4') class ImportTestMixin(object): """ Tests to run for both course and library import pages. @@ -271,6 +277,7 @@ class ImportTestMixin(object): self.import_page.wait_for_tasks(fail_on='Updating') +@attr('shard_4') class TestEntranceExamCourseImport(ImportTestMixin, StudioCourseTest): """ Tests the Course import page @@ -316,6 +323,7 @@ class TestEntranceExamCourseImport(ImportTestMixin, StudioCourseTest): ) +@attr('shard_4') class TestCourseImport(ImportTestMixin, StudioCourseTest): """ Tests the Course import page @@ -357,6 +365,7 @@ class TestCourseImport(ImportTestMixin, StudioCourseTest): self.assertEqual(self.import_page.header_text, 'Course Import') +@attr('shard_4') class TestLibraryImport(ImportTestMixin, StudioLibraryTest): """ Tests the Library import page diff --git a/common/test/acceptance/tests/studio/test_studio_container.py b/common/test/acceptance/tests/studio/test_studio_container.py index 040e9ef84d..fdf0630552 100644 --- a/common/test/acceptance/tests/studio/test_studio_container.py +++ b/common/test/acceptance/tests/studio/test_studio_container.py @@ -312,6 +312,7 @@ class EditContainerTest(NestedVerticalTest): self.assertEqual(component.student_content, "modified content") +@attr('shard_3') class EditVisibilityModalTest(ContainerBase): """ Tests of the visibility settings modal for components on the unit @@ -1041,6 +1042,7 @@ class UnitPublishingTest(ContainerBase): # self.assertEqual('discussion', self.courseware.xblock_component_type(1)) +@attr('shard_3') class DisplayNameTest(ContainerBase): """ Test consistent use of display_name_with_default @@ -1077,6 +1079,7 @@ class DisplayNameTest(ContainerBase): self.assertEqual(container.name, title_on_unit_page) +@attr('shard_3') class ProblemCategoryTabsTest(ContainerBase): """ Test to verify tabs in problem category. diff --git a/common/test/acceptance/tests/studio/test_studio_outline.py b/common/test/acceptance/tests/studio/test_studio_outline.py index 8cd1935910..715bff1653 100644 --- a/common/test/acceptance/tests/studio/test_studio_outline.py +++ b/common/test/acceptance/tests/studio/test_studio_outline.py @@ -1755,6 +1755,7 @@ class DeprecationWarningMessageTest(CourseOutlineTest): ) +@attr('shard_4') class SelfPacedOutlineTest(CourseOutlineTest): """Test the course outline for a self-paced course.""" diff --git a/common/test/acceptance/tests/studio/test_studio_settings_details.py b/common/test/acceptance/tests/studio/test_studio_settings_details.py index 7c361b7230..aff356cfb9 100644 --- a/common/test/acceptance/tests/studio/test_studio_settings_details.py +++ b/common/test/acceptance/tests/studio/test_studio_settings_details.py @@ -2,6 +2,7 @@ Acceptance tests for Studio's Settings Details pages """ from datetime import datetime, timedelta +from nose.plugins.attrib import attr from unittest import skip from .base_studio_test import StudioCourseTest @@ -18,6 +19,7 @@ from ..helpers import ( ) +@attr('shard_4') class StudioSettingsDetailsTest(StudioCourseTest): """Base class for settings and details page tests.""" @@ -35,6 +37,7 @@ class StudioSettingsDetailsTest(StudioCourseTest): self.assertTrue(self.settings_detail.is_browser_on_page()) +@attr('shard_4') class SettingsMilestonesTest(StudioSettingsDetailsTest): """ Tests for milestones feature in Studio's settings tab @@ -201,6 +204,7 @@ class SettingsMilestonesTest(StudioSettingsDetailsTest): )) +@attr('shard_4') class CoursePacingTest(StudioSettingsDetailsTest): """Tests for setting a course to self-paced.""" From 01d8a9bcad06e41241d040c43b4862addaa65cac Mon Sep 17 00:00:00 2001 From: Christine Lytwynec Date: Wed, 18 Nov 2015 15:04:54 -0500 Subject: [PATCH 10/10] Fix paths for diff quality jshint comparison --- pavelib/quality.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pavelib/quality.py b/pavelib/quality.py index f570d623fd..b7aa130554 100644 --- a/pavelib/quality.py +++ b/pavelib/quality.py @@ -273,8 +273,8 @@ def run_jshint(options): _prepare_report_dir(jshint_report_dir) sh( - "jshint {root} --config .jshintrc >> {jshint_report}".format( - root=Env.REPO_ROOT, jshint_report=jshint_report + "jshint . --config .jshintrc >> {jshint_report}".format( + jshint_report=jshint_report ), ignore_error=True )