From 3e52b8fad19aab2c0c5b8992897987835e160269 Mon Sep 17 00:00:00 2001 From: Matt Drayer Date: Thu, 4 Jun 2015 15:31:16 -0400 Subject: [PATCH] mattdrayer/SOL-947: Refactor Web/HTML certificate URL patterns --- .../contentstore/views/tests/test_assets.py | 7 ++ .../views/tests/test_certificates.py | 8 +++ cms/envs/bok_choy.env.json | 1 + cms/templates/widgets/header.html | 7 +- cms/urls.py | 21 +++--- .../test/acceptance/pages/studio/settings.py | 64 +++++++++++++------ .../pages/studio/settings_certificates.py | 4 +- .../studio/test_studio_settings_details.py | 5 +- lms/djangoapps/certificates/views.py | 6 +- test_root/uploads/.gitignore | 1 + 10 files changed, 90 insertions(+), 34 deletions(-) diff --git a/cms/djangoapps/contentstore/views/tests/test_assets.py b/cms/djangoapps/contentstore/views/tests/test_assets.py index 9f9ce4b4c7..9bd55c9d50 100644 --- a/cms/djangoapps/contentstore/views/tests/test_assets.py +++ b/cms/djangoapps/contentstore/views/tests/test_assets.py @@ -6,7 +6,10 @@ from io import BytesIO from pytz import UTC from PIL import Image import json + from django.conf import settings +from django.test.utils import override_settings + from contentstore.tests.utils import CourseTestCase from contentstore.views import assets from contentstore.utils import reverse_course_url @@ -28,7 +31,11 @@ TEST_DATA_DIR = settings.COMMON_TEST_DATA_ROOT MAX_FILE_SIZE = settings.MAX_ASSET_UPLOAD_FILE_SIZE_IN_MB * 1000 ** 2 +FEATURES_WITH_CERTS_ENABLED = settings.FEATURES.copy() +FEATURES_WITH_CERTS_ENABLED['CERTIFICATES_HTML_VIEW'] = True + +@override_settings(FEATURES=FEATURES_WITH_CERTS_ENABLED) class AssetsTestCase(CourseTestCase): """ Parent class for all asset tests. diff --git a/cms/djangoapps/contentstore/views/tests/test_certificates.py b/cms/djangoapps/contentstore/views/tests/test_certificates.py index d2394ee2fc..7997e9b575 100644 --- a/cms/djangoapps/contentstore/views/tests/test_certificates.py +++ b/cms/djangoapps/contentstore/views/tests/test_certificates.py @@ -6,6 +6,9 @@ Group Configuration Tests. import json import mock +from django.conf import settings +from django.test.utils import override_settings + from opaque_keys.edx.keys import AssetKey from opaque_keys.edx.locations import AssetLocation @@ -20,6 +23,9 @@ from contentstore.views.certificates import CertificateManager from django.test.utils import override_settings from contentstore.utils import get_lms_link_for_certificate_web_view +FEATURES_WITH_CERTS_ENABLED = settings.FEATURES.copy() +FEATURES_WITH_CERTS_ENABLED['CERTIFICATES_HTML_VIEW'] = True + CERTIFICATE_JSON = { u'name': u'Test certificate', u'description': u'Test description', @@ -185,6 +191,7 @@ class CertificatesBaseTestCase(object): # pylint: disable=no-member +@override_settings(FEATURES=FEATURES_WITH_CERTS_ENABLED) class CertificatesListHandlerTestCase(CourseTestCase, CertificatesBaseTestCase, HelperMethods): """ Test cases for certificates_list_handler. @@ -323,6 +330,7 @@ class CertificatesListHandlerTestCase(CourseTestCase, CertificatesBaseTestCase, self.assertNotEqual(new_certificate.get('id'), prev_certificate.get('id')) +@override_settings(FEATURES=FEATURES_WITH_CERTS_ENABLED) class CertificatesDetailHandlerTestCase(CourseTestCase, CertificatesBaseTestCase, HelperMethods): """ Test cases for CertificatesDetailHandlerTestCase. diff --git a/cms/envs/bok_choy.env.json b/cms/envs/bok_choy.env.json index af6f6002d8..88624fb85f 100644 --- a/cms/envs/bok_choy.env.json +++ b/cms/envs/bok_choy.env.json @@ -65,6 +65,7 @@ "FEATURES": { "AUTH_USE_OPENID_PROVIDER": true, "CERTIFICATES_ENABLED": true, + "CERTIFICATES_HTML_VIEW": true, "DASHBOARD_SHARE_SETTINGS": { "CUSTOM_COURSE_URLS": true }, diff --git a/cms/templates/widgets/header.html b/cms/templates/widgets/header.html index 2c4b611f6c..222b93c549 100644 --- a/cms/templates/widgets/header.html +++ b/cms/templates/widgets/header.html @@ -1,5 +1,6 @@ <%namespace name='static' file='../static_content.html'/> <%! + from django.conf import settings from django.core.urlresolvers import reverse from django.utils.translation import ugettext as _ from contentstore.context_processors import doc_url @@ -35,8 +36,12 @@ grading_url = reverse('contentstore.views.grading_handler', kwargs={'course_key_string': unicode(course_key)}) advanced_settings_url = reverse('contentstore.views.advanced_settings_handler', kwargs={'course_key_string': unicode(course_key)}) tabs_url = reverse('contentstore.views.tabs_handler', kwargs={'course_key_string': unicode(course_key)}) - certificates_url = reverse('contentstore.views.certificates.certificates_list_handler', kwargs={'course_key_string': unicode(course_key)}) %> + % if settings.FEATURES.get("CERTIFICATES_HTML_VIEW") and context_course: + <% + certificates_url = reverse('contentstore.views.certificates.certificates_list_handler', kwargs={'course_key_string': unicode(course_key)}) + %> + % endif

${_("Current Course:")} diff --git a/cms/urls.py b/cms/urls.py index ff6d98c7fb..2c69ccfea1 100644 --- a/cms/urls.py +++ b/cms/urls.py @@ -112,14 +112,6 @@ urlpatterns += patterns( url(r'^group_configurations/{}$'.format(settings.COURSE_KEY_PATTERN), 'group_configurations_list_handler'), url(r'^group_configurations/{}/(?P\d+)(/)?(?P\d+)?$'.format( settings.COURSE_KEY_PATTERN), 'group_configurations_detail_handler'), - url(r'^certificates/{}$'.format(settings.COURSE_KEY_PATTERN), 'certificates.certificates_list_handler'), - url(r'^certificates/{}/(?P\d+)/signatories/(?P\d+)?$'.format( - settings.COURSE_KEY_PATTERN), 'certificates.signatory_detail_handler'), - url(r'^certificates/{}/(?P\d+)?$'.format(settings.COURSE_KEY_PATTERN), - 'certificates.certificates_detail_handler'), - url(r'^certificates/activation/{}/'.format(settings.COURSE_KEY_PATTERN), - 'certificates.certificate_activation_handler'), - url(r'^api/val/v0/', include('edxval.urls')), ) @@ -178,6 +170,19 @@ if settings.FEATURES.get('ENTRANCE_EXAMS'): url(r'^course/{}/entrance_exam/?$'.format(settings.COURSE_KEY_PATTERN), 'contentstore.views.entrance_exam'), ) +# Enable Web/HTML Certificates +if settings.FEATURES.get('CERTIFICATES_HTML_VIEW'): + urlpatterns += ( + url(r'^certificates/activation/{}/'.format(settings.COURSE_KEY_PATTERN), + 'contentstore.views.certificates.certificate_activation_handler'), + url(r'^certificates/{}/(?P\d+)/signatories/(?P\d+)?$'.format( + settings.COURSE_KEY_PATTERN), 'contentstore.views.certificates.signatory_detail_handler'), + url(r'^certificates/{}/(?P\d+)?$'.format(settings.COURSE_KEY_PATTERN), + 'contentstore.views.certificates.certificates_detail_handler'), + url(r'^certificates/{}$'.format(settings.COURSE_KEY_PATTERN), + 'contentstore.views.certificates.certificates_list_handler') + ) + if settings.DEBUG: try: from .urls_dev import urlpatterns as dev_urlpatterns diff --git a/common/test/acceptance/pages/studio/settings.py b/common/test/acceptance/pages/studio/settings.py index f3c0bc0a77..4391e43289 100644 --- a/common/test/acceptance/pages/studio/settings.py +++ b/common/test/acceptance/pages/studio/settings.py @@ -16,6 +16,9 @@ class SettingsPage(CoursePage): url_path = "settings/details" + ################ + # Helpers + ################ def is_browser_on_page(self): return self.q(css='body.view-settings').present @@ -38,6 +41,9 @@ class SettingsPage(CoursePage): results = self.get_elements(css_selector=css_selector) return results[0] if results else None + ################ + # Properties + ################ @property def pre_requisite_course_options(self): """ @@ -60,25 +66,6 @@ class SettingsPage(CoursePage): """ return self.get_element('#alert-confirmation-title') - def require_entrance_exam(self, required=True): - """ - Set the entrance exam requirement via the checkbox. - """ - checkbox = self.entrance_exam_field - selected = checkbox.is_selected() - if required and not selected: - checkbox.click() - self.wait_for_element_visibility( - '#entrance-exam-minimum-score-pct', - 'Entrance exam minimum score percent is visible' - ) - if not required and selected: - checkbox.click() - self.wait_for_element_invisibility( - '#entrance-exam-minimum-score-pct', - 'Entrance exam minimum score percent is invisible' - ) - @property def course_license(self): """ @@ -123,6 +110,45 @@ class SettingsPage(CoursePage): raise Exception("Invalid license name: {name}".format(name=license_name)) button.click() + ################ + # Waits + ################ + def wait_for_prerequisite_course_options(self): + """ + Ensure the pre_requisite_course_options dropdown selector is displayed + """ + EmptyPromise( + lambda: self.q(css="#pre-requisite-course").present, + 'Prerequisite course dropdown selector is displayed' + ).fulfill() + + ################ + # Clicks + ################ + + ################ + # Workflows + ################ + + def require_entrance_exam(self, required=True): + """ + Set the entrance exam requirement via the checkbox. + """ + checkbox = self.entrance_exam_field + selected = checkbox.is_selected() + if required and not selected: + checkbox.click() + self.wait_for_element_visibility( + '#entrance-exam-minimum-score-pct', + 'Entrance exam minimum score percent is visible' + ) + if not required and selected: + checkbox.click() + self.wait_for_element_invisibility( + '#entrance-exam-minimum-score-pct', + 'Entrance exam minimum score percent is invisible' + ) + def save_changes(self, wait_for_confirmation=True): """ Clicks save button, waits for confirmation unless otherwise specified diff --git a/common/test/acceptance/pages/studio/settings_certificates.py b/common/test/acceptance/pages/studio/settings_certificates.py index a74c5bdb40..6d521efbaa 100644 --- a/common/test/acceptance/pages/studio/settings_certificates.py +++ b/common/test/acceptance/pages/studio/settings_certificates.py @@ -299,9 +299,11 @@ class Certificate(object): Delete the certificate """ self.wait_for_certificate_delete_button() + self.find_css('.actions .delete').first.click() self.page.wait_for_confirmation_prompt() - self.find_css('.action-primary').first.click() + self.page.q(css='a.button.action-primary').first.click() + self.page.q(css='a.button.action-primary').first.click() self.page.wait_for_ajax() 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 5a3d65b7c8..cebb7d6206 100644 --- a/common/test/acceptance/tests/studio/test_studio_settings_details.py +++ b/common/test/acceptance/tests/studio/test_studio_settings_details.py @@ -69,6 +69,7 @@ class SettingsMilestonesTest(StudioCourseTest): # Refresh the page to load the new course fixture and populate the prrequisite course dropdown # Then select the prerequisite course and save the changes self.settings_detail.refresh_page() + self.settings_detail.wait_for_prerequisite_course_options() select_option_by_value( browser_query=self.settings_detail.pre_requisite_course_options, value=pre_requisite_course_id @@ -79,8 +80,9 @@ class SettingsMilestonesTest(StudioCourseTest): self.settings_detail.alert_confirmation_title.text ) - # Refresh the page again to confirm the prerequisite course selection is properly reflected + # Refresh the page again and confirm the prerequisite course selection is properly reflected self.settings_detail.refresh_page() + self.settings_detail.wait_for_prerequisite_course_options() self.assertTrue(is_option_value_selected( browser_query=self.settings_detail.pre_requisite_course_options, value=pre_requisite_course_id @@ -99,6 +101,7 @@ class SettingsMilestonesTest(StudioCourseTest): # Refresh the page again to confirm the None selection is properly reflected self.settings_detail.refresh_page() + self.settings_detail.wait_for_prerequisite_course_options() self.assertTrue(is_option_value_selected( browser_query=self.settings_detail.pre_requisite_course_options, value='' diff --git a/lms/djangoapps/certificates/views.py b/lms/djangoapps/certificates/views.py index 162611c103..f4b0e55a71 100644 --- a/lms/djangoapps/certificates/views.py +++ b/lms/djangoapps/certificates/views.py @@ -553,8 +553,9 @@ def render_html_view(request, user_id, course_id): # 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, request.GET.get('preview')) - if active_configuration is None and request.GET.get('preview') is None: + if active_configuration is None: return render_to_response(invalid_template_path, context) else: context['certificate_data'] = active_configuration @@ -568,9 +569,6 @@ def render_html_view(request, user_id, course_id): # Append/Override the existing view context values with any course-specific static values from Advanced Settings context.update(course.cert_html_view_overrides) - # Override further with any course-specific static values - context.update(course.cert_html_view_overrides) - # FINALLY, generate and send the output the client return render_to_response("certificates/valid.html", context) diff --git a/test_root/uploads/.gitignore b/test_root/uploads/.gitignore index ce360105a0..39fc5e825c 100644 --- a/test_root/uploads/.gitignore +++ b/test_root/uploads/.gitignore @@ -1,2 +1,3 @@ *.csv *.jpg +*.png