diff --git a/cms/envs/bok_choy.env.json b/cms/envs/bok_choy.env.json index 44cd55ef73..943a80bf1e 100644 --- a/cms/envs/bok_choy.env.json +++ b/cms/envs/bok_choy.env.json @@ -90,7 +90,7 @@ "LOCAL_LOGLEVEL": "INFO", "LOGGING_ENV": "sandbox", "LOG_DIR": "** OVERRIDDEN **", - "MEDIA_URL": "", + "MEDIA_URL": "/media/", "MKTG_URL_LINK_MAP": {}, "PLATFORM_NAME": "edX", "SERVER_EMAIL": "devops@example.com", diff --git a/common/djangoapps/config_models/admin.py b/common/djangoapps/config_models/admin.py index a0753ff33c..3718ad3131 100644 --- a/common/djangoapps/config_models/admin.py +++ b/common/djangoapps/config_models/admin.py @@ -6,6 +6,7 @@ from django.forms import models from django.contrib import admin from django.contrib.admin import ListFilter from django.core.cache import caches, InvalidCacheBackendError +from django.core.files.base import File from django.core.urlresolvers import reverse from django.http import HttpResponseRedirect from django.shortcuts import get_object_or_404 @@ -178,7 +179,16 @@ class KeyedConfigurationModelAdmin(ConfigurationModelAdmin): get = request.GET.copy() source_id = int(get.pop('source')[0]) source = get_object_or_404(self.model, pk=source_id) - get.update(models.model_to_dict(source)) + source_dict = models.model_to_dict(source) + for field_name, field_value in source_dict.items(): + # read files into request.FILES, if: + # * user hasn't ticked the "clear" checkbox + # * user hasn't uploaded a new file + if field_value and isinstance(field_value, File): + clear_checkbox_name = '{0}-clear'.format(field_name) + if request.POST.get(clear_checkbox_name) != 'on': + request.FILES.setdefault(field_name, field_value) + get[field_name] = field_value request.GET = get # Call our grandparent's add_view, skipping the parent code # because the parent code has a different way to prepopulate new configuration entries diff --git a/common/djangoapps/course_modes/models.py b/common/djangoapps/course_modes/models.py index 1d1deb8787..a7409bad23 100644 --- a/common/djangoapps/course_modes/models.py +++ b/common/djangoapps/course_modes/models.py @@ -398,7 +398,7 @@ class CourseMode(models.Model): @classmethod def has_verified_mode(cls, course_mode_dict): - """Check whether the modes for a course allow a student to pursue a verfied certificate. + """Check whether the modes for a course allow a student to pursue a verified certificate. Args: course_mode_dict (dictionary mapping course mode slugs to Modes) diff --git a/common/djangoapps/third_party_auth/admin.py b/common/djangoapps/third_party_auth/admin.py index 02cc865325..3491be6eee 100644 --- a/common/djangoapps/third_party_auth/admin.py +++ b/common/djangoapps/third_party_auth/admin.py @@ -37,7 +37,8 @@ class SAMLProviderConfigAdmin(KeyedConfigurationModelAdmin): """ Don't show every single field in the admin change list """ return ( 'name', 'enabled', 'backend_name', 'entity_id', 'metadata_source', - 'has_data', 'icon_class', 'change_date', 'changed_by', 'edit_link' + 'has_data', 'icon_class', 'icon_image', 'change_date', + 'changed_by', 'edit_link' ) def has_data(self, inst): @@ -104,6 +105,7 @@ class LTIProviderConfigAdmin(KeyedConfigurationModelAdmin): exclude = ( 'icon_class', + 'icon_image', 'secondary', ) diff --git a/common/djangoapps/third_party_auth/migrations/0002_schema__provider_icon_image.py b/common/djangoapps/third_party_auth/migrations/0002_schema__provider_icon_image.py new file mode 100644 index 0000000000..073cddf69c --- /dev/null +++ b/common/djangoapps/third_party_auth/migrations/0002_schema__provider_icon_image.py @@ -0,0 +1,44 @@ +# -*- coding: utf-8 -*- +from __future__ import unicode_literals + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('third_party_auth', '0001_initial'), + ] + + operations = [ + migrations.AddField( + model_name='ltiproviderconfig', + name='icon_image', + field=models.FileField(help_text=b'If there is no Font Awesome icon available for this provider, upload a custom image. SVG images are recommended as they can scale to any size.', upload_to=b'', blank=True), + ), + migrations.AddField( + model_name='oauth2providerconfig', + name='icon_image', + field=models.FileField(help_text=b'If there is no Font Awesome icon available for this provider, upload a custom image. SVG images are recommended as they can scale to any size.', upload_to=b'', blank=True), + ), + migrations.AddField( + model_name='samlproviderconfig', + name='icon_image', + field=models.FileField(help_text=b'If there is no Font Awesome icon available for this provider, upload a custom image. SVG images are recommended as they can scale to any size.', upload_to=b'', blank=True), + ), + migrations.AlterField( + model_name='ltiproviderconfig', + name='icon_class', + field=models.CharField(default=b'fa-sign-in', help_text=b'The Font Awesome (or custom) icon class to use on the login button for this provider. Examples: fa-google-plus, fa-facebook, fa-linkedin, fa-sign-in, fa-university', max_length=50, blank=True), + ), + migrations.AlterField( + model_name='oauth2providerconfig', + name='icon_class', + field=models.CharField(default=b'fa-sign-in', help_text=b'The Font Awesome (or custom) icon class to use on the login button for this provider. Examples: fa-google-plus, fa-facebook, fa-linkedin, fa-sign-in, fa-university', max_length=50, blank=True), + ), + migrations.AlterField( + model_name='samlproviderconfig', + name='icon_class', + field=models.CharField(default=b'fa-sign-in', help_text=b'The Font Awesome (or custom) icon class to use on the login button for this provider. Examples: fa-google-plus, fa-facebook, fa-linkedin, fa-sign-in, fa-university', max_length=50, blank=True), + ), + ] diff --git a/common/djangoapps/third_party_auth/models.py b/common/djangoapps/third_party_auth/models.py index b699cbe25e..5200a92b71 100644 --- a/common/djangoapps/third_party_auth/models.py +++ b/common/djangoapps/third_party_auth/models.py @@ -70,12 +70,25 @@ class ProviderConfig(ConfigurationModel): Abstract Base Class for configuring a third_party_auth provider """ icon_class = models.CharField( - max_length=50, default='fa-sign-in', + max_length=50, + blank=True, + default='fa-sign-in', help_text=( 'The Font Awesome (or custom) icon class to use on the login button for this provider. ' 'Examples: fa-google-plus, fa-facebook, fa-linkedin, fa-sign-in, fa-university' ), ) + # We use a FileField instead of an ImageField here because ImageField + # doesn't support SVG. This means we don't get any image validation, but + # that should be fine because only trusted users should be uploading these + # anyway. + icon_image = models.FileField( + blank=True, + help_text=( + 'If there is no Font Awesome icon available for this provider, upload a custom image. ' + 'SVG images are recommended as they can scale to any size.' + ), + ) name = models.CharField(max_length=50, blank=False, help_text="Name of this provider (shown to users)") secondary = models.BooleanField( default=False, @@ -109,6 +122,12 @@ class ProviderConfig(ConfigurationModel): app_label = "third_party_auth" abstract = True + def clean(self): + """ Ensure that either `icon_class` or `icon_image` is set """ + super(ProviderConfig, self).clean() + if bool(self.icon_class) == bool(self.icon_image): + raise ValidationError('Either an icon class or an icon image must be given (but not both)') + @property def provider_id(self): """ Unique string key identifying this provider. Must be URL and css class friendly. """ @@ -500,9 +519,15 @@ class LTIProviderConfig(ProviderConfig): """ prefix = 'lti' backend_name = 'lti' - icon_class = None # This provider is not visible to users - secondary = False # This provider is not visible to users - accepts_logins = False # LTI login cannot be initiated by the tool provider + + # This provider is not visible to users + icon_class = None + icon_image = None + secondary = False + + # LTI login cannot be initiated by the tool provider + accepts_logins = False + KEY_FIELDS = ('lti_consumer_key', ) lti_consumer_key = models.CharField( diff --git a/common/djangoapps/third_party_auth/tests/specs/test_azuread.py b/common/djangoapps/third_party_auth/tests/specs/test_azuread.py new file mode 100644 index 0000000000..680983250a --- /dev/null +++ b/common/djangoapps/third_party_auth/tests/specs/test_azuread.py @@ -0,0 +1,46 @@ +"""Integration tests for Azure Active Directory / Microsoft Account provider.""" + +from third_party_auth.tests.specs import base + + +# pylint: disable=test-inherits-tests +class AzureADOauth2IntegrationTest(base.Oauth2IntegrationTest): + """Integration tests for Azure Active Directory / Microsoft Account provider.""" + + def setUp(self): + super(AzureADOauth2IntegrationTest, self).setUp() + self.provider = self.configure_azure_ad_provider( + enabled=True, + key='azure_ad_oauth2_key', + secret='azure_ad_oauth2_secret', + ) + + TOKEN_RESPONSE_DATA = { + 'exp': 1234590302, + 'nbf': 1234586402, + 'iat': 1234586402, + 'expires_on': '1234590302', + 'ver': '1.0', + 'access_token': 'access_token_value', + 'expires_in': '3599', + 'id_token': 'id_token_value', + 'token_type': 'Bearer', + 'refresh_token': 'REFRESH1234567890', + 'iss': 'https://sts.windows.net/abcdefgh-1234-5678-900a-0aa0a00aa0aa/', + 'ipaddr': '123.123.123.123', + } + USER_RESPONSE_DATA = { + 'oid': 'abcdefgh-1234-5678-900a-0aa0a00aa0aa', + 'aud': 'abcdefgh-1234-5678-900a-0aa0a00aa0aa', + 'tid': 'abcdefgh-1234-5678-900a-0aa0a00aa0aa', + 'amr': ['pwd'], + 'unique_name': 'email_value@example.com', + 'upn': 'email_value@example.com', + 'family_name': 'family_name_value', + 'name': 'name_value', + 'given_name': 'given_name_value', + 'sub': 'aBC_ab12345678h94CSgP1lTYJCHATGQDAcfg8jSOck', + } + + def get_username(self): + return self.get_response_data().get('name') diff --git a/common/djangoapps/third_party_auth/tests/test_admin.py b/common/djangoapps/third_party_auth/tests/test_admin.py new file mode 100644 index 0000000000..9ae56b5a86 --- /dev/null +++ b/common/djangoapps/third_party_auth/tests/test_admin.py @@ -0,0 +1,85 @@ +""" +Tests third_party_auth admin views +""" +import unittest + +from django.conf import settings +from django.contrib.admin.sites import AdminSite +from django.core.urlresolvers import reverse +from django.core.files.uploadedfile import SimpleUploadedFile +from django.forms import models + +from student.tests.factories import UserFactory +from third_party_auth.admin import OAuth2ProviderConfigAdmin +from third_party_auth.models import OAuth2ProviderConfig +from third_party_auth.tests import testutil + + +# This is necessary because cms does not implement third party auth +@unittest.skipUnless(settings.FEATURES.get('ENABLE_THIRD_PARTY_AUTH'), 'third party auth not enabled') +class Oauth2ProviderConfigAdminTest(testutil.TestCase): + """ + Tests for oauth2 provider config admin + """ + def test_oauth2_provider_edit_icon_image(self): + """ + Test that we can update an OAuth provider's icon image from the admin + form. + + OAuth providers are updated using KeyedConfigurationModelAdmin, which + updates models by adding a new instance that replaces the old one, + instead of editing the old instance directly. + + Updating the icon image is tricky here because + KeyedConfigurationModelAdmin copies data over from the previous + version by injecting its attributes into request.GET, but the icon + ends up in request.FILES. We need to ensure that the value is + prepopulated correctly, and that we can clear and update the image. + """ + # Login as a super user + user = UserFactory.create(is_staff=True, is_superuser=True) + user.save() + self.client.login(username=user.username, password='test') + + # Get baseline provider count + providers = OAuth2ProviderConfig.objects.all() + pcount = len(providers) + + # Create a provider + provider1 = self.configure_dummy_provider( + enabled=True, + icon_class='', + icon_image=SimpleUploadedFile('icon.svg', ''), + ) + + # Get the provider instance with active flag + providers = OAuth2ProviderConfig.objects.all() + self.assertEquals(len(providers), 1) + self.assertEquals(providers[pcount].id, provider1.id) + + # Edit the provider via the admin edit link + admin = OAuth2ProviderConfigAdmin(provider1, AdminSite()) + # pylint: disable=protected-access + update_url = reverse('admin:{}_{}_add'.format(admin.model._meta.app_label, admin.model._meta.model_name)) + update_url += "?source={}".format(provider1.pk) + + # Remove the icon_image from the POST data, to simulate unchanged icon_image + post_data = models.model_to_dict(provider1) + del post_data['icon_image'] + + # Change the name, to verify POST + post_data['name'] = 'Another name' + + # Post the edit form: expecting redirect + response = self.client.post(update_url, post_data) + self.assertEquals(response.status_code, 302) + + # Editing the existing provider creates a new provider instance + providers = OAuth2ProviderConfig.objects.all() + self.assertEquals(len(providers), pcount + 2) + self.assertEquals(providers[pcount].id, provider1.id) + provider2 = providers[pcount + 1] + + # Ensure the icon_image was preserved on the new provider instance + self.assertEquals(provider2.icon_image, provider1.icon_image) + self.assertEquals(provider2.name, post_data['name']) diff --git a/common/djangoapps/third_party_auth/tests/testutil.py b/common/djangoapps/third_party_auth/tests/testutil.py index 36ac1c698c..fa9ef60bf1 100644 --- a/common/djangoapps/third_party_auth/tests/testutil.py +++ b/common/djangoapps/third_party_auth/tests/testutil.py @@ -13,6 +13,7 @@ import django.test from mako.template import Template import mock import os.path +from storages.backends.overwrite import OverwriteStorage from third_party_auth.models import ( OAuth2ProviderConfig, @@ -52,6 +53,17 @@ class FakeDjangoSettings(object): class ThirdPartyAuthTestMixin(object): """ Helper methods useful for testing third party auth functionality """ + def setUp(self, *args, **kwargs): + # Django's FileSystemStorage will rename files if they already exist. + # This storage backend overwrites files instead, which makes it easier + # to make assertions about filenames. + icon_image_field = OAuth2ProviderConfig._meta.get_field('icon_image') # pylint: disable=protected-access + patch = mock.patch.object(icon_image_field, 'storage', OverwriteStorage()) + patch.start() + self.addCleanup(patch.stop) + + super(ThirdPartyAuthTestMixin, self).setUp(*args, **kwargs) + def tearDown(self): config_cache.clear() super(ThirdPartyAuthTestMixin, self).tearDown() @@ -112,6 +124,16 @@ class ThirdPartyAuthTestMixin(object): kwargs.setdefault("secret", "test") return cls.configure_oauth_provider(**kwargs) + @classmethod + def configure_azure_ad_provider(cls, **kwargs): + """ Update the settings for the Azure AD third party auth provider/backend """ + kwargs.setdefault("name", "Azure AD") + kwargs.setdefault("backend_name", "azuread-oauth2") + kwargs.setdefault("icon_class", "fa-azuread") + kwargs.setdefault("key", "test") + kwargs.setdefault("secret", "test") + return cls.configure_oauth_provider(**kwargs) + @classmethod def configure_twitter_provider(cls, **kwargs): """ Update the settings for the Twitter third party auth provider/backend """ @@ -124,7 +146,7 @@ class ThirdPartyAuthTestMixin(object): @classmethod def configure_dummy_provider(cls, **kwargs): - """ Update the settings for the Twitter third party auth provider/backend """ + """ Update the settings for the Dummy third party auth provider/backend """ kwargs.setdefault("name", "Dummy") kwargs.setdefault("backend_name", "dummy") return cls.configure_oauth_provider(**kwargs) diff --git a/common/static/sass/_mixins.scss b/common/static/sass/_mixins.scss index 09f2e1b982..cf781c3b50 100644 --- a/common/static/sass/_mixins.scss +++ b/common/static/sass/_mixins.scss @@ -24,6 +24,7 @@ // * +Content - Text Wrap - Extend // * +Content - Text Truncate - Extend // * +Icon - Font-Awesome - Extend +// * +Icon - SSO icon images // +Font Sizing - Mixin // ==================== @@ -448,3 +449,17 @@ padding: 0; margin: 0; } + + +// * +Icon - SSO icon images +// ==================== + +%sso-icon { + .icon-image { + width: auto; + height: auto; + max-height: 1.4em; + max-width: 1.4em; + margin-top: -2px; + } +} diff --git a/common/test/acceptance/tests/lms/test_lms.py b/common/test/acceptance/tests/lms/test_lms.py index 1999c385ae..2ab32a4640 100644 --- a/common/test/acceptance/tests/lms/test_lms.py +++ b/common/test/acceptance/tests/lms/test_lms.py @@ -166,8 +166,11 @@ class LoginFromCombinedPageTest(UniqueCourseTest): # Create a user account email, password = self._create_unique_user() - # Navigate to the login page and try to log in using "Dummy" provider + # Navigate to the login page self.login_page.visit() + self.assertScreenshot('#login .login-providers', 'login-providers') + + # Try to log in using "Dummy" provider self.login_page.click_third_party_dummy_provider() # The user will be redirected somewhere and then back to the login page: @@ -206,6 +209,7 @@ class LoginFromCombinedPageTest(UniqueCourseTest): # We should now be redirected to the login page self.login_page.wait_for_page() self.assertIn("Would you like to sign in using your Dummy credentials?", self.login_page.hinted_login_prompt) + self.assertScreenshot('#hinted-login-form', 'hinted-login') self.login_page.click_third_party_dummy_provider() # We should now be redirected to the course page @@ -329,8 +333,11 @@ class RegisterFromCombinedPageTest(UniqueCourseTest): Test that we can register using third party credentials, and that the third party account gets linked to the edX account. """ - # Navigate to the register page and try to authenticate using the "Dummy" provider + # Navigate to the register page self.register_page.visit() + self.assertScreenshot('#register .login-providers', 'register-providers') + + # Try to authenticate using the "Dummy" provider self.register_page.click_third_party_dummy_provider() # The user will be redirected somewhere and then back to the register page: diff --git a/common/test/acceptance/tests/lms/test_lms_instructor_dashboard.py b/common/test/acceptance/tests/lms/test_lms_instructor_dashboard.py index dd79a53310..df4e0a2455 100644 --- a/common/test/acceptance/tests/lms/test_lms_instructor_dashboard.py +++ b/common/test/acceptance/tests/lms/test_lms_instructor_dashboard.py @@ -3,9 +3,8 @@ End-to-end tests for the LMS Instructor Dashboard. """ -import time +import ddt -from flaky import flaky from nose.plugins.attrib import attr from bok_choy.promise import EmptyPromise @@ -652,6 +651,7 @@ class DataDownloadsTest(BaseInstructorDashboardTest): @attr('shard_7') +@ddt.ddt class CertificatesTest(BaseInstructorDashboardTest): """ Tests for Certificates functionality on instructor dashboard. @@ -907,6 +907,36 @@ class CertificatesTest(BaseInstructorDashboardTest): self.certificates_section.message.text ) + @ddt.data( + ('Test \nNotes', 'Test Notes'), + ('Notes', 'Notes'), + ) + @ddt.unpack + def test_notes_escaped_in_add_certificate_exception(self, notes, expected_notes): + """ + Scenario: On the Certificates tab of the Instructor Dashboard, Instructor can add new certificate + exception to list. + + Given that I am on the Certificates tab on the Instructor Dashboard + When I fill in student username and notes (which contains character which are needed to be escaped) + and click 'Add Exception' button, then new certificate exception should be visible in + certificate exceptions list. + """ + # Add a student to Certificate exception list + self.certificates_section.add_certificate_exception(self.user_name, notes) + self.assertIn(self.user_name, self.certificates_section.last_certificate_exception.text) + self.assertIn(expected_notes, self.certificates_section.last_certificate_exception.text) + + # Revisit Page & verify that added exceptions are also synced with backend + self.certificates_section.refresh() + + # Wait for the certificate exception section to render + self.certificates_section.wait_for_certificate_exceptions_section() + + # Validate certificate exception synced with server is visible in certificate exceptions list + self.assertIn(self.user_name, self.certificates_section.last_certificate_exception.text) + self.assertIn(expected_notes, self.certificates_section.last_certificate_exception.text) + @attr('shard_7') class CertificateInvalidationTest(BaseInstructorDashboardTest): diff --git a/common/test/db_fixtures/third_party_auth.json b/common/test/db_fixtures/third_party_auth.json index 3042ebbb66..9b4fad7bff 100644 --- a/common/test/db_fixtures/third_party_auth.json +++ b/common/test/db_fixtures/third_party_auth.json @@ -8,6 +8,7 @@ "changed_by": null, "name": "Google", "icon_class": "fa-google-plus", + "icon_image": null, "backend_name": "google-oauth2", "key": "test", "secret": "test", @@ -23,6 +24,7 @@ "changed_by": null, "name": "Facebook", "icon_class": "fa-facebook", + "icon_image": null, "backend_name": "facebook", "key": "test", "secret": "test", @@ -37,7 +39,8 @@ "change_date": "2001-02-03T04:05:06Z", "changed_by": null, "name": "Dummy", - "icon_class": "fa-sign-in", + "icon_class": "", + "icon_image": "test-icon.png", "backend_name": "dummy", "key": "", "secret": "", diff --git a/lms/djangoapps/bulk_email/tasks.py b/lms/djangoapps/bulk_email/tasks.py index b4308ef2d0..1c894be49c 100644 --- a/lms/djangoapps/bulk_email/tasks.py +++ b/lms/djangoapps/bulk_email/tasks.py @@ -216,11 +216,6 @@ def perform_delegate_email_batches(entry_id, course_id, task_input, action_name) # Fetch the course object. course = get_course(course_id) - if course is None: - msg = u"Task %s: course not found: %s" - log.error(msg, task_id, course_id) - raise ValueError(msg % (task_id, course_id)) - # Get arguments that will be passed to every subtask. to_option = email_obj.to_option global_email_context = _get_course_email_context(course) @@ -403,11 +398,32 @@ def _get_source_address(course_id, course_title): # For the email address, get the course. Then make sure that it can be used # in an email address, by substituting a '_' anywhere a non-(ascii, period, or dash) # character appears. - from_addr = u'"{0}" Course Staff <{1}-{2}>'.format( - course_title_no_quotes, - re.sub(r"[^\w.-]", '_', course_id.course), - settings.BULK_EMAIL_DEFAULT_FROM_EMAIL - ) + course_name = re.sub(r"[^\w.-]", '_', course_id.course) + + from_addr_format = u'"{course_title}" Course Staff <{course_name}-{from_email}>' + + def format_address(course_title_no_quotes): + """ + Partial function for formatting the from_addr. Since + `course_title_no_quotes` may be truncated to make sure the returned + string has fewer than 320 characters, we define this function to make + it easy to determine quickly what the max length is for + `course_title_no_quotes`. + """ + return from_addr_format.format( + course_title=course_title_no_quotes, + course_name=course_name, + from_email=settings.BULK_EMAIL_DEFAULT_FROM_EMAIL, + ) + + from_addr = format_address(course_title_no_quotes) + + # If it's longer than 320 characters, reformat, but with the course name + # rather than course title. Amazon SES's from address field appears to have a maximum + # length of 320. + if len(from_addr) >= 320: + from_addr = format_address(course_name) + return from_addr diff --git a/lms/djangoapps/bulk_email/tests/test_email.py b/lms/djangoapps/bulk_email/tests/test_email.py index fe95b073b3..d1a32e641d 100644 --- a/lms/djangoapps/bulk_email/tests/test_email.py +++ b/lms/djangoapps/bulk_email/tests/test_email.py @@ -20,7 +20,8 @@ from instructor_task.subtasks import update_subtask_status from student.roles import CourseStaffRole from student.models import CourseEnrollment from student.tests.factories import CourseEnrollmentFactory, UserFactory -from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase +from xmodule.modulestore import ModuleStoreEnum +from xmodule.modulestore.tests.django_utils import SharedModuleStoreTestCase from xmodule.modulestore.tests.factories import CourseFactory STAFF_COUNT = 3 @@ -44,44 +45,77 @@ class MockCourseEmailResult(object): return mock_update_subtask_status -class EmailSendFromDashboardTestCase(ModuleStoreTestCase): +class EmailSendFromDashboardTestCase(SharedModuleStoreTestCase): """ Test that emails send correctly. """ - @patch.dict(settings.FEATURES, {'ENABLE_INSTRUCTOR_EMAIL': True, 'REQUIRE_COURSE_EMAIL_AUTH': False}) - def setUp(self): - super(EmailSendFromDashboardTestCase, self).setUp() - course_title = u"ẗëṡẗ title ｲ乇丂ｲ ﾶ乇丂丂ﾑg乇 ｷo尺 ﾑﾚﾚ тэѕт мэѕѕаБэ" - self.course = CourseFactory.create(display_name=course_title) - + def create_staff_and_instructor(self): + """ + Creates one instructor and several course staff for self.course. Assigns + them to self.instructor (single user) and self.staff (list of users), + respectively. + """ self.instructor = InstructorFactory(course_key=self.course.id) - # Create staff - self.staff = [StaffFactory(course_key=self.course.id) - for _ in xrange(STAFF_COUNT)] + self.staff = [ + StaffFactory(course_key=self.course.id) for __ in xrange(STAFF_COUNT) + ] - # Create students + def create_students(self): + """ + Creates users and enrolls them in self.course. Assigns these users to + self.students. + """ self.students = [UserFactory() for _ in xrange(STUDENT_COUNT)] for student in self.students: CourseEnrollmentFactory.create(user=student, course_id=self.course.id) + def login_as_user(self, user): + """ + Log in self.client as user. + """ + self.client.login(username=user.username, password="test") + + @patch.dict(settings.FEATURES, {'ENABLE_INSTRUCTOR_EMAIL': True, 'REQUIRE_COURSE_EMAIL_AUTH': False}) + def goto_instructor_dash_email_view(self): + """ + Goes to the instructor dashboard to verify that the email section is + there. + """ + url = reverse('instructor_dashboard', kwargs={'course_id': unicode(self.course.id)}) + # Response loads the whole instructor dashboard, so no need to explicitly + # navigate to a particular email section + response = self.client.get(url) + email_section = '
' + # If this fails, it is likely because ENABLE_INSTRUCTOR_EMAIL is set to False + self.assertIn(email_section, response.content) + + @classmethod + def setUpClass(cls): + super(EmailSendFromDashboardTestCase, cls).setUpClass() + course_title = u"ẗëṡẗ title ｲ乇丂ｲ ﾶ乇丂丂ﾑg乇 ｷo尺 ﾑﾚﾚ тэѕт мэѕѕаБэ" + cls.course = CourseFactory.create( + display_name=course_title, + default_store=ModuleStoreEnum.Type.split + ) + + def setUp(self): + super(EmailSendFromDashboardTestCase, self).setUp() + self.create_staff_and_instructor() + self.create_students() + # load initial content (since we don't run migrations as part of tests): call_command("loaddata", "course_email_template.json") - self.client.login(username=self.instructor.username, password="test") + self.login_as_user(self.instructor) - # Pull up email view on instructor dashboard - self.url = reverse('instructor_dashboard', kwargs={'course_id': self.course.id.to_deprecated_string()}) - # Response loads the whole instructor dashboard, so no need to explicitly - # navigate to a particular email section - response = self.client.get(self.url) - email_section = '
' - # If this fails, it is likely because ENABLE_INSTRUCTOR_EMAIL is set to False - self.assertTrue(email_section in response.content) - self.send_mail_url = reverse('send_email', kwargs={'course_id': self.course.id.to_deprecated_string()}) + self.goto_instructor_dash_email_view() + self.send_mail_url = reverse( + 'send_email', kwargs={'course_id': unicode(self.course.id)} + ) self.success_content = { - 'course_id': self.course.id.to_deprecated_string(), + 'course_id': unicode(self.course.id), 'success': True, } @@ -130,6 +164,13 @@ class TestEmailSendFromDashboardMockedHtmlToText(EmailSendFromDashboardTestCase) self.assertEqual(len(mail.outbox[0].to), 1) self.assertEquals(mail.outbox[0].to[0], self.instructor.email) self.assertEquals(mail.outbox[0].subject, 'test subject for myself') + self.assertEquals( + mail.outbox[0].from_email, + u'"{course_display_name}" Course Staff <{course_name}-no-reply@example.com>'.format( + course_display_name=self.course.display_name, + course_name=self.course.id.course + ) + ) def test_send_to_staff(self): """ @@ -268,6 +309,42 @@ class TestEmailSendFromDashboardMockedHtmlToText(EmailSendFromDashboardTestCase) [self.instructor.email] + [s.email for s in self.staff] + [s.email for s in self.students] ) + def test_long_course_display_name(self): + """ + This test tests that courses with exorbitantly large display names + can still send emails, since it appears that 320 appears to be the + character length limit of from emails for Amazon SES. + """ + test_email = { + 'action': 'Send email', + 'send_to': 'myself', + 'subject': 'test subject for self', + 'message': 'test message for self' + } + + # make very long display_name for course + long_name = u"x" * 321 + course = CourseFactory.create( + display_name=long_name, number="bulk_email_course_name" + ) + instructor = InstructorFactory(course_key=course.id) + + self.login_as_user(instructor) + send_mail_url = reverse('send_email', kwargs={'course_id': unicode(course.id)}) + response = self.client.post(send_mail_url, test_email) + self.assertTrue(json.loads(response.content)['success']) + + self.assertEqual(len(mail.outbox), 1) + from_email = mail.outbox[0].from_email + + self.assertEqual( + from_email, + u'"{course_name}" Course Staff <{course_name}-no-reply@example.com>'.format( + course_name=course.id.course + ) + ) + self.assertEqual(len(from_email), 83) + @override_settings(BULK_EMAIL_EMAILS_PER_TASK=3) @patch('bulk_email.tasks.update_subtask_status') def test_chunked_queries_send_numerous_emails(self, email_mock): diff --git a/lms/djangoapps/certificates/views/support.py b/lms/djangoapps/certificates/views/support.py index 65d92adacf..89af213b74 100644 --- a/lms/djangoapps/certificates/views/support.py +++ b/lms/djangoapps/certificates/views/support.py @@ -260,5 +260,10 @@ def generate_certificate_for_user(request): return HttpResponseBadRequest(msg) # Attempt to generate certificate - generate_certificates_for_students(request, params["course_key"], students=[params["user"]]) + generate_certificates_for_students( + request, + params["course_key"], + student_set="specific_student", + specific_student_id=params["user"].id + ) return HttpResponse(200) diff --git a/lms/djangoapps/courseware/date_summary.py b/lms/djangoapps/courseware/date_summary.py index 4d9c1b9dc7..e2b998b55b 100644 --- a/lms/djangoapps/courseware/date_summary.py +++ b/lms/djangoapps/courseware/date_summary.py @@ -217,6 +217,27 @@ class VerifiedUpgradeDeadlineDate(DateSummary): return ecommerce_service.checkout_page_url(course_mode.sku) return reverse('verify_student_upgrade_and_verify', args=(self.course.id,)) + @property + def is_enabled(self): + """ + Whether or not this summary block should be shown. + + By default, the summary is only shown if it has date and the date is in the + future and the user's enrollment is in upsell modes + """ + is_enabled = super(VerifiedUpgradeDeadlineDate, self).is_enabled + if not is_enabled: + return False + + enrollment_mode, is_active = CourseEnrollment.enrollment_mode_for_user(self.user, self.course.id) + + # Return `true` if user is not enrolled in course + if enrollment_mode is None and is_active is None: + return True + + # Show the summary if user enrollment is in which allow user to upsell + return is_active and enrollment_mode in CourseMode.UPSELL_TO_VERIFIED_MODES + @lazy def date(self): try: diff --git a/lms/djangoapps/courseware/tests/test_date_summary.py b/lms/djangoapps/courseware/tests/test_date_summary.py index 8682749c70..69683d7602 100644 --- a/lms/djangoapps/courseware/tests/test_date_summary.py +++ b/lms/djangoapps/courseware/tests/test_date_summary.py @@ -42,7 +42,9 @@ class CourseDateSummaryTest(SharedModuleStoreTestCase): days_till_start=1, days_till_end=14, days_till_upgrade_deadline=4, + enroll_user=True, enrollment_mode=CourseMode.VERIFIED, + course_min_price=100, days_till_verification_deadline=14, verification_status=None, sku=None @@ -64,11 +66,13 @@ class CourseDateSummaryTest(SharedModuleStoreTestCase): course_id=self.course.id, mode_slug=enrollment_mode, expiration_datetime=now + timedelta(days=days_till_upgrade_deadline), + min_price=course_min_price, sku=sku ) + + if enroll_user: + enrollment_mode = enrollment_mode or CourseMode.DEFAULT_MODE_SLUG CourseEnrollmentFactory.create(course_id=self.course.id, user=self.user, mode=enrollment_mode) - else: - CourseEnrollmentFactory.create(course_id=self.course.id, user=self.user) if days_till_verification_deadline is not None: VerificationDeadline.objects.create( @@ -95,21 +99,36 @@ class CourseDateSummaryTest(SharedModuleStoreTestCase): self.assertEqual(set(type(b) for b in blocks), set(expected_blocks)) @ddt.data( - # Before course starts - ({}, (CourseEndDate, CourseStartDate, TodaysDate, VerificationDeadlineDate, VerifiedUpgradeDeadlineDate)), - # After course end + # Verified enrollment with no photo-verification before course start + ({}, (CourseEndDate, CourseStartDate, TodaysDate, VerificationDeadlineDate)), + # Verified enrollment with `approved` photo-verification after course end ({'days_till_start': -10, 'days_till_end': -5, 'days_till_upgrade_deadline': -6, 'days_till_verification_deadline': -5, 'verification_status': 'approved'}, (TodaysDate, CourseEndDate)), - # No course end date + # Verified enrollment with `expired` photo-verification during course run + ({'days_till_start': -10, + 'verification_status': 'expired'}, + (TodaysDate, CourseEndDate, VerificationDeadlineDate)), + # Verified enrollment with `approved` photo-verification during course run + ({'days_till_start': -10, + 'verification_status': 'approved'}, + (TodaysDate, CourseEndDate)), + # Audit enrollment and non-upsell course. + ({'days_till_start': -10, + 'days_till_upgrade_deadline': None, + 'days_till_verification_deadline': None, + 'course_min_price': 0, + 'enrollment_mode': CourseMode.AUDIT}, + (TodaysDate, CourseEndDate)), + # Verified enrollment with *NO* course end date ({'days_till_end': None}, - (CourseStartDate, TodaysDate, VerificationDeadlineDate, VerifiedUpgradeDeadlineDate)), - # During course run + (CourseStartDate, TodaysDate, VerificationDeadlineDate)), + # Verified enrollment with no photo-verification during course run ({'days_till_start': -1}, - (TodaysDate, CourseEndDate, VerificationDeadlineDate, VerifiedUpgradeDeadlineDate)), + (TodaysDate, CourseEndDate, VerificationDeadlineDate)), # Verification approved ({'days_till_start': -10, 'days_till_upgrade_deadline': -1, @@ -117,13 +136,26 @@ class CourseDateSummaryTest(SharedModuleStoreTestCase): 'verification_status': 'approved'}, (TodaysDate, CourseEndDate)), # After upgrade deadline - ({'days_till_start': -10, 'days_till_upgrade_deadline': -1}, + ({'days_till_start': -10, + 'days_till_upgrade_deadline': -1}, (TodaysDate, CourseEndDate, VerificationDeadlineDate)), # After verification deadline ({'days_till_start': -10, 'days_till_upgrade_deadline': -2, 'days_till_verification_deadline': -1}, - (TodaysDate, CourseEndDate, VerificationDeadlineDate)) + (TodaysDate, CourseEndDate, VerificationDeadlineDate)), + # Un-enrolled user before course start + ({'enroll_user': False}, + (CourseStartDate, TodaysDate, CourseEndDate, VerifiedUpgradeDeadlineDate)), + # Un-enrolled user during course run + ({'days_till_start': -1, + 'enroll_user': False}, + (TodaysDate, CourseEndDate, VerifiedUpgradeDeadlineDate)), + # Un-enrolled user after course end. + ({'enroll_user': False, + 'days_till_start': -10, + 'days_till_end': -5}, + (TodaysDate, CourseEndDate, VerifiedUpgradeDeadlineDate)), ) @ddt.unpack def test_enabled_block_types(self, course_options, expected_blocks): diff --git a/lms/djangoapps/instructor/tests/test_certificates.py b/lms/djangoapps/instructor/tests/test_certificates.py index 5367e871c7..b8e8e0317c 100644 --- a/lms/djangoapps/instructor/tests/test_certificates.py +++ b/lms/djangoapps/instructor/tests/test_certificates.py @@ -720,7 +720,6 @@ class GenerateCertificatesInstructorApiTest(SharedModuleStoreTestCase): response = self.client.post( url, - data=json.dumps([self.certificate_exception]), content_type='application/json' ) # Assert Success @@ -736,24 +735,49 @@ class GenerateCertificatesInstructorApiTest(SharedModuleStoreTestCase): u"Certificate generation started for white listed students." ) - def test_generate_certificate_exceptions_invalid_user_list_error(self): + def test_generate_certificate_exceptions_whitelist_not_generated(self): """ - Test generate certificates exceptions api endpoint returns error - when called with certificate exceptions with empty 'user_id' field + Test generate certificates exceptions api endpoint returns success + when calling with new certificate exception. """ url = reverse( 'generate_certificate_exceptions', kwargs={'course_id': unicode(self.course.id), 'generate_for': 'new'} ) - # assign empty user_id - self.certificate_exception.update({'user_id': ''}) + response = self.client.post( + url, + content_type='application/json' + ) + + # Assert Success + self.assertEqual(response.status_code, 200) + + res_json = json.loads(response.content) + + # Assert Request is successful + self.assertTrue(res_json['success']) + # Assert Message + self.assertEqual( + res_json['message'], + u"Certificate generation started for white listed students." + ) + + def test_generate_certificate_exceptions_generate_for_incorrect_value(self): + """ + Test generate certificates exceptions api endpoint returns error + when calling with generate_for without 'new' or 'all' value. + """ + url = reverse( + 'generate_certificate_exceptions', + kwargs={'course_id': unicode(self.course.id), 'generate_for': ''} + ) response = self.client.post( url, - data=json.dumps([self.certificate_exception]), content_type='application/json' ) + # Assert Failure self.assertEqual(response.status_code, 400) @@ -764,7 +788,7 @@ class GenerateCertificatesInstructorApiTest(SharedModuleStoreTestCase): # Assert Message self.assertEqual( res_json['message'], - u"Invalid data, user_id must be present for all certificate exceptions." + u'Invalid data, generate_for must be "new" or "all".' ) diff --git a/lms/djangoapps/instructor/views/api.py b/lms/djangoapps/instructor/views/api.py index 9cd7fb2e71..cca2c8ca1c 100644 --- a/lms/djangoapps/instructor/views/api.py +++ b/lms/djangoapps/instructor/views/api.py @@ -3032,41 +3032,24 @@ def generate_certificate_exceptions(request, course_id, generate_for=None): """ course_key = CourseKey.from_string(course_id) - try: - certificate_white_list = json.loads(request.body) - except ValueError: - return JsonResponse({ - 'success': False, - 'message': _('Invalid Json data, Please refresh the page and then try again.') - }, status=400) - - users = [exception.get('user_id', False) for exception in certificate_white_list] - if generate_for == 'all': # Generate Certificates for all white listed students - students = User.objects.filter( - certificatewhitelist__course_id=course_key, - certificatewhitelist__whitelist=True - ) - elif not all(users): - # Invalid data, user_id must be present for all certificate exceptions + students = 'all_whitelisted' + + elif generate_for == 'new': + students = 'whitelisted_not_generated' + + else: + # Invalid data, generate_for must be present for all certificate exceptions return JsonResponse( { 'success': False, - 'message': _('Invalid data, user_id must be present for all certificate exceptions.'), + 'message': _('Invalid data, generate_for must be "new" or "all".'), }, status=400 ) - else: - students = User.objects.filter( - id__in=users, - certificatewhitelist__course_id=course_key, - certificatewhitelist__whitelist=True - ) - if students: - # generate certificates for students if 'students' list is not empty - instructor_task.api.generate_certificates_for_students(request, course_key, students=students) + instructor_task.api.generate_certificates_for_students(request, course_key, student_set=students) response_payload = { 'success': True, @@ -3275,8 +3258,10 @@ def re_validate_certificate(request, course_key, generated_certificate): certificate_invalidation.deactivate() # We need to generate certificate only for a single student here - students = [certificate_invalidation.generated_certificate.user] - instructor_task.api.generate_certificates_for_students(request, course_key, students=students) + student = certificate_invalidation.generated_certificate.user + instructor_task.api.generate_certificates_for_students( + request, course_key, student_set="specific_student", specific_student_id=student.id + ) def validate_request_data_and_get_certificate(certificate_invalidation, course_key): diff --git a/lms/djangoapps/instructor_task/api.py b/lms/djangoapps/instructor_task/api.py index 1f7b50847f..10939226f4 100644 --- a/lms/djangoapps/instructor_task/api.py +++ b/lms/djangoapps/instructor_task/api.py @@ -45,6 +45,13 @@ from bulk_email.models import CourseEmail from util import milestones_helpers +class SpecificStudentIdMissingError(Exception): + """ + Exception indicating that a student id was not provided when generating a certificate for a specific student. + """ + pass + + def get_running_instructor_tasks(course_id): """ Returns a query of InstructorTask objects of running tasks for a given course. @@ -437,17 +444,34 @@ def submit_export_ora2_data(request, course_key): return submit_task(request, task_type, task_class, course_key, task_input, task_key) -def generate_certificates_for_students(request, course_key, students=None): # pylint: disable=invalid-name +def generate_certificates_for_students(request, course_key, student_set=None, specific_student_id=None): # pylint: disable=invalid-name """ - Submits a task to generate certificates for given students enrolled in the course or - all students if argument 'students' is None + Submits a task to generate certificates for given students enrolled in the course. + + Arguments: + course_key : Course Key + student_set : Semantic for student collection for certificate generation. + Options are: + 'all_whitelisted': All Whitelisted students. + 'whitelisted_not_generated': Whitelisted students which does not got certificates yet. + 'specific_student': Single student for certificate generation. + specific_student_id : Student ID when student_set is 'specific_student' Raises AlreadyRunningError if certificates are currently being generated. + Raises SpecificStudentIdMissingError if student_set is 'specific_student' and specific_student_id is 'None' """ - if students: - task_type = 'generate_certificates_certain_student' - students = [student.id for student in students] - task_input = {'students': students} + if student_set: + task_type = 'generate_certificates_student_set' + task_input = {'student_set': student_set} + + if student_set == 'specific_student': + task_type = 'generate_certificates_certain_student' + if specific_student_id is None: + raise SpecificStudentIdMissingError( + "Attempted to generate certificate for a single student, " + "but no specific student id provided" + ) + task_input.update({'specific_student_id': specific_student_id}) else: task_type = 'generate_certificates_all_student' task_input = {} @@ -466,22 +490,16 @@ def generate_certificates_for_students(request, course_key, students=None): # p return instructor_task -def regenerate_certificates(request, course_key, statuses_to_regenerate, students=None): +def regenerate_certificates(request, course_key, statuses_to_regenerate): """ - Submits a task to regenerate certificates for given students enrolled in the course or - all students if argument 'students' is None. + Submits a task to regenerate certificates for given students enrolled in the course. Regenerate Certificate only if the status of the existing generated certificate is in 'statuses_to_regenerate' list passed in the arguments. Raises AlreadyRunningError if certificates are currently being generated. """ - if students: - task_type = 'regenerate_certificates_certain_student' - students = [student.id for student in students] - task_input = {'students': students} - else: - task_type = 'regenerate_certificates_all_student' - task_input = {} + task_type = 'regenerate_certificates_all_student' + task_input = {} task_input.update({"statuses_to_regenerate": statuses_to_regenerate}) task_class = generate_certificates diff --git a/lms/djangoapps/instructor_task/tasks_helper.py b/lms/djangoapps/instructor_task/tasks_helper.py index a4f50953ad..bd32135472 100644 --- a/lms/djangoapps/instructor_task/tasks_helper.py +++ b/lms/djangoapps/instructor_task/tasks_helper.py @@ -1409,30 +1409,56 @@ def generate_students_certificates( json column, otherwise generate certificates for all enrolled students. """ start_time = time() - enrolled_students = CourseEnrollment.objects.users_enrolled_in(course_id) + students_to_generate_certs_for = CourseEnrollment.objects.users_enrolled_in(course_id) - students = task_input.get('students', None) + student_set = task_input.get('student_set') + if student_set == 'all_whitelisted': + # Generate Certificates for all white listed students. + students_to_generate_certs_for = students_to_generate_certs_for.filter( + certificatewhitelist__course_id=course_id, + certificatewhitelist__whitelist=True + ) - if students is not None: - enrolled_students = enrolled_students.filter(id__in=students) + elif student_set == 'whitelisted_not_generated': + # All Whitelisted students + students_to_generate_certs_for = students_to_generate_certs_for.filter( + certificatewhitelist__course_id=course_id, + certificatewhitelist__whitelist=True + ) - task_progress = TaskProgress(action_name, enrolled_students.count(), start_time) + # Whitelisted students which got certificates already. + certificate_generated_students = GeneratedCertificate.objects.filter( # pylint: disable=no-member + course_id=course_id, + ) + certificate_generated_students_ids = set(certificate_generated_students.values_list('user_id', flat=True)) + + students_to_generate_certs_for = students_to_generate_certs_for.exclude( + id__in=certificate_generated_students_ids + ) + + elif student_set == "specific_student": + specific_student_id = task_input.get('specific_student_id') + students_to_generate_certs_for = students_to_generate_certs_for.filter(id=specific_student_id) + + task_progress = TaskProgress(action_name, students_to_generate_certs_for.count(), start_time) current_step = {'step': 'Calculating students already have certificates'} task_progress.update_task_state(extra_meta=current_step) statuses_to_regenerate = task_input.get('statuses_to_regenerate', []) - if students is not None and not statuses_to_regenerate: + if student_set is not None and not statuses_to_regenerate: # We want to skip 'filtering students' only when students are given and statuses to regenerate are not - students_require_certs = enrolled_students + students_require_certs = students_to_generate_certs_for else: - students_require_certs = students_require_certificate(course_id, enrolled_students, statuses_to_regenerate) + students_require_certs = students_require_certificate( + course_id, students_to_generate_certs_for, statuses_to_regenerate + ) if statuses_to_regenerate: # Mark existing generated certificates as 'unavailable' before regenerating # We need to call this method after "students_require_certificate" otherwise "students_require_certificate" # would return no results. - invalidate_generated_certificates(course_id, enrolled_students, statuses_to_regenerate) + invalidate_generated_certificates(course_id, students_to_generate_certs_for, statuses_to_regenerate) task_progress.skipped = task_progress.total - len(students_require_certs) diff --git a/lms/djangoapps/instructor_task/tests/test_api.py b/lms/djangoapps/instructor_task/tests/test_api.py index 5dcd09769a..2e1b3648dd 100644 --- a/lms/djangoapps/instructor_task/tests/test_api.py +++ b/lms/djangoapps/instructor_task/tests/test_api.py @@ -24,6 +24,7 @@ from instructor_task.api import ( generate_certificates_for_students, regenerate_certificates, submit_export_ora2_data, + SpecificStudentIdMissingError, ) from instructor_task.api_helper import AlreadyRunningError @@ -295,6 +296,18 @@ class InstructorTaskCourseSubmitTest(TestReportMixin, InstructorTaskCourseTestCa ) self._test_resubmission(api_call) + def test_certificate_generation_no_specific_student_id(self): + """ + Raises ValueError when student_set is 'specific_student' and 'specific_student_id' is None. + """ + with self.assertRaises(SpecificStudentIdMissingError): + generate_certificates_for_students( + self.create_task_request(self.instructor), + self.course.id, + student_set='specific_student', + specific_student_id=None + ) + def test_certificate_generation_history(self): """ Tests that a new record is added whenever certificate generation/regeneration task is submitted. diff --git a/lms/djangoapps/instructor_task/tests/test_tasks_helper.py b/lms/djangoapps/instructor_task/tests/test_tasks_helper.py index 72a354652d..1d05a83dec 100644 --- a/lms/djangoapps/instructor_task/tests/test_tasks_helper.py +++ b/lms/djangoapps/instructor_task/tests/test_tasks_helper.py @@ -1628,8 +1628,7 @@ class TestCertificateGeneration(InstructorTaskModuleTestCase): Verify that certificates generated for all eligible students enrolled in a course. """ # create 10 students - students = [self.create_student(username='student_{}'.format(i), email='student_{}@example.com'.format(i)) - for i in xrange(1, 11)] + students = self._create_students(10) # mark 2 students to have certificates generated already for student in students[:2]: @@ -1644,40 +1643,157 @@ class TestCertificateGeneration(InstructorTaskModuleTestCase): for student in students[2:7]: CertificateWhitelistFactory.create(user=student, course_id=self.course.id, whitelist=True) - current_task = Mock() - current_task.update_state = Mock() - instructor_task = Mock() - instructor_task.task_input = json.dumps({'students': None}) + task_input = {'student_set': None} + expected_results = { + 'action_name': 'certificates generated', + 'total': 10, + 'attempted': 8, + 'succeeded': 5, + 'failed': 3, + 'skipped': 2 + } + with self.assertNumQueries(214): - with patch('instructor_task.tasks_helper._get_current_task') as mock_current_task: - mock_current_task.return_value = current_task - with patch('capa.xqueue_interface.XQueueInterface.send_to_queue') as mock_queue: - mock_queue.return_value = (0, "Successfully queued") - with patch('instructor_task.models.InstructorTask.objects.get') as instructor_task_object: - instructor_task_object.return_value = instructor_task - result = generate_students_certificates( - None, None, self.course.id, {}, 'certificates generated' - ) - self.assertDictContainsSubset( - { - 'action_name': 'certificates generated', - 'total': 10, - 'attempted': 8, - 'succeeded': 5, - 'failed': 3, - 'skipped': 2 - }, - result + self.assertCertificatesGenerated(task_input, expected_results) + + def test_certificate_generation_all_whitelisted(self): + """ + Verify that certificates generated for all white-listed students when using semantic task_input as + `all_whitelisted`. + """ + # create 5 students + students = self._create_students(5) + + # white-list 5 students + for student in students: + CertificateWhitelistFactory.create(user=student, course_id=self.course.id, whitelist=True) + + task_input = {'student_set': 'all_whitelisted'} + expected_results = { + 'action_name': 'certificates generated', + 'total': 5, + 'attempted': 5, + 'succeeded': 5, + 'failed': 0, + 'skipped': 0 + } + self.assertCertificatesGenerated(task_input, expected_results) + + def test_certificate_generation_whitelist_already_generated(self): + """ + Verify that certificates generated for all white-listed students having certifcates already when using + semantic task_input as `all_whitelisted`. + """ + # create 5 students + students = self._create_students(5) + + # white-list 5 students + for student in students: + CertificateWhitelistFactory.create(user=student, course_id=self.course.id, whitelist=True) + + # mark 5 students to have certificates generated already + for student in students: + GeneratedCertificateFactory.create( + user=student, + course_id=self.course.id, + status=CertificateStatuses.downloadable, + mode='honor' + ) + + task_input = {'student_set': 'all_whitelisted'} + expected_results = { + 'action_name': 'certificates generated', + 'total': 5, + 'attempted': 5, + 'succeeded': 5, + 'failed': 0, + 'skipped': 0 + } + self.assertCertificatesGenerated(task_input, expected_results) + + def test_certificate_generation_whitelisted_not_generated(self): + """ + Verify that certificates only generated for those students which does not have certificates yet when + using semantic task_input as `whitelisted_not_generated`. + """ + # create 5 students + students = self._create_students(5) + + # mark 2 students to have certificates generated already + for student in students[:2]: + GeneratedCertificateFactory.create( + user=student, + course_id=self.course.id, + status=CertificateStatuses.downloadable, + mode='honor' + ) + + # white-list 5 students + for student in students: + CertificateWhitelistFactory.create(user=student, course_id=self.course.id, whitelist=True) + + task_input = {'student_set': 'whitelisted_not_generated'} + + expected_results = { + 'action_name': 'certificates generated', + 'total': 3, + 'attempted': 3, + 'succeeded': 3, + 'failed': 0, + 'skipped': 0 + } + self.assertCertificatesGenerated( + task_input, + expected_results ) + def test_certificate_generation_specific_student(self): + """ + Tests generating a certificate for a specific student. + """ + student = self.create_student(username="Hamnet", email="ham@ardenforest.co.uk") + CertificateWhitelistFactory.create(user=student, course_id=self.course.id, whitelist=True) + task_input = { + 'student_set': 'specific_student', + 'specific_student_id': student.id + } + expected_results = { + 'action_name': 'certificates generated', + 'total': 1, + 'attempted': 1, + 'succeeded': 1, + 'failed': 0, + 'skipped': 0, + } + self.assertCertificatesGenerated(task_input, expected_results) + + def test_specific_student_not_enrolled(self): + """ + Tests generating a certificate for a specific student if that student + is not enrolled in the course. + """ + student = self.create_student(username="jacques", email="antlers@ardenforest.co.uk") + task_input = { + 'student_set': 'specific_student', + 'specific_student_id': student.id + } + expected_results = { + 'action_name': 'certificates generated', + 'total': 1, + 'attempted': 1, + 'succeeded': 0, + 'failed': 1, + 'skipped': 0, + } + self.assertCertificatesGenerated(task_input, expected_results) + def test_certificate_regeneration_for_statuses_to_regenerate(self): """ Verify that certificates are regenerated for all eligible students enrolled in a course whose generated certificate statuses lies in the list 'statuses_to_regenerate' given in task_input. """ # create 10 students - students = [self.create_student(username='student_{}'.format(i), email='student_{}@example.com'.format(i)) - for i in xrange(1, 11)] + students = self._create_students(10) # mark 2 students to have certificates generated already for student in students[:2]: @@ -1710,31 +1826,22 @@ class TestCertificateGeneration(InstructorTaskModuleTestCase): for student in students[:7]: CertificateWhitelistFactory.create(user=student, course_id=self.course.id, whitelist=True) - current_task = Mock() - current_task.update_state = Mock() - # Certificates should be regenerated for students having generated certificates with status # 'downloadable' or 'error' which are total of 5 students in this test case task_input = {'statuses_to_regenerate': [CertificateStatuses.downloadable, CertificateStatuses.error]} - with patch('instructor_task.tasks_helper._get_current_task') as mock_current_task: - mock_current_task.return_value = current_task - with patch('capa.xqueue_interface.XQueueInterface.send_to_queue') as mock_queue: - mock_queue.return_value = (0, "Successfully queued") - result = generate_students_certificates( - None, None, self.course.id, task_input, 'certificates generated' - ) + expected_results = { + 'action_name': 'certificates generated', + 'total': 10, + 'attempted': 5, + 'succeeded': 5, + 'failed': 0, + 'skipped': 5 + } - self.assertDictContainsSubset( - { - 'action_name': 'certificates generated', - 'total': 10, - 'attempted': 5, - 'succeeded': 5, - 'failed': 0, - 'skipped': 5 - }, - result + self.assertCertificatesGenerated( + task_input, + expected_results ) def test_certificate_regeneration_with_expected_failures(self): @@ -1746,8 +1853,7 @@ class TestCertificateGeneration(InstructorTaskModuleTestCase): default_grade = '-1' # create 10 students - students = [self.create_student(username='student_{}'.format(i), email='student_{}@example.com'.format(i)) - for i in xrange(1, 11)] + students = self._create_students(10) # mark 2 students to have certificates generated already for student in students[:2]: @@ -1796,32 +1902,21 @@ class TestCertificateGeneration(InstructorTaskModuleTestCase): for student in students[:7]: CertificateWhitelistFactory.create(user=student, course_id=self.course.id, whitelist=True) - current_task = Mock() - current_task.update_state = Mock() - # Regenerated certificates for students having generated certificates with status # 'deleted' or 'generating' task_input = {'statuses_to_regenerate': [CertificateStatuses.deleted, CertificateStatuses.generating]} - with patch('instructor_task.tasks_helper._get_current_task') as mock_current_task: - mock_current_task.return_value = current_task - with patch('capa.xqueue_interface.XQueueInterface.send_to_queue') as mock_queue: - mock_queue.return_value = (0, "Successfully queued") - result = generate_students_certificates( - None, None, self.course.id, task_input, 'certificates generated' - ) + expected_results = { + 'action_name': 'certificates generated', + 'total': 10, + 'attempted': 5, + 'succeeded': 2, + 'failed': 3, + 'skipped': 5 + } + + self.assertCertificatesGenerated(task_input, expected_results) - self.assertDictContainsSubset( - { - 'action_name': 'certificates generated', - 'total': 10, - 'attempted': 5, - 'succeeded': 2, - 'failed': 3, - 'skipped': 5 - }, - result - ) generated_certificates = GeneratedCertificate.eligible_certificates.filter( user__in=students, course_id=self.course.id, @@ -1852,8 +1947,7 @@ class TestCertificateGeneration(InstructorTaskModuleTestCase): default_grade = '-1' # create 10 students - students = [self.create_student(username='student_{}'.format(i), email='student_{}@example.com'.format(i)) - for i in xrange(1, 11)] + students = self._create_students(10) # mark 2 students to have certificates generated already for student in students[:2]: @@ -1899,9 +1993,6 @@ class TestCertificateGeneration(InstructorTaskModuleTestCase): for student in students[:]: CertificateWhitelistFactory.create(user=student, course_id=self.course.id, whitelist=True) - current_task = Mock() - current_task.update_state = Mock() - # Regenerated certificates for students having generated certificates with status # 'downloadable', 'error' or 'generating' task_input = { @@ -1912,24 +2003,18 @@ class TestCertificateGeneration(InstructorTaskModuleTestCase): ] } - with patch('instructor_task.tasks_helper._get_current_task') as mock_current_task: - mock_current_task.return_value = current_task - with patch('capa.xqueue_interface.XQueueInterface.send_to_queue') as mock_queue: - mock_queue.return_value = (0, "Successfully queued") - result = generate_students_certificates( - None, None, self.course.id, task_input, 'certificates generated' - ) + expected_results = { + 'action_name': 'certificates generated', + 'total': 10, + 'attempted': 8, + 'succeeded': 8, + 'failed': 0, + 'skipped': 2 + } - self.assertDictContainsSubset( - { - 'action_name': 'certificates generated', - 'total': 10, - 'attempted': 8, - 'succeeded': 8, - 'failed': 0, - 'skipped': 2 - }, - result + self.assertCertificatesGenerated( + task_input, + expected_results ) generated_certificates = GeneratedCertificate.eligible_certificates.filter( @@ -1963,8 +2048,7 @@ class TestCertificateGeneration(InstructorTaskModuleTestCase): Verify that certificates are regenerated for all students passed in task_input. """ # create 10 students - students = [self.create_student(username='student_{}'.format(i), email='student_{}@example.com'.format(i)) - for i in xrange(1, 11)] + students = self._create_students(10) # mark 2 students to have certificates generated already for student in students[:2]: @@ -2006,12 +2090,27 @@ class TestCertificateGeneration(InstructorTaskModuleTestCase): for student in students[:7]: CertificateWhitelistFactory.create(user=student, course_id=self.course.id, whitelist=True) - current_task = Mock() - current_task.update_state = Mock() - # Certificates should be regenerated for students having generated certificates with status # 'downloadable' or 'error' which are total of 5 students in this test case - task_input = {'students': [student.id for student in students]} + task_input = {'student_set': "all_whitelisted"} + + expected_results = { + 'action_name': 'certificates generated', + 'total': 7, + 'attempted': 7, + 'succeeded': 7, + 'failed': 0, + 'skipped': 0, + } + + self.assertCertificatesGenerated(task_input, expected_results) + + def assertCertificatesGenerated(self, task_input, expected_results): + """ + Generate certificates for the given task_input and compare with expected_results. + """ + current_task = Mock() + current_task.update_state = Mock() with patch('instructor_task.tasks_helper._get_current_task') as mock_current_task: mock_current_task.return_value = current_task @@ -2022,17 +2121,22 @@ class TestCertificateGeneration(InstructorTaskModuleTestCase): ) self.assertDictContainsSubset( - { - 'action_name': 'certificates generated', - 'total': 10, - 'attempted': 10, - 'succeeded': 7, - 'failed': 3, - 'skipped': 0, - }, + expected_results, result ) + def _create_students(self, number_of_students): + """ + Create Students for course. + """ + return [ + self.create_student( + username='student_{}'.format(index), + email='student_{}@example.com'.format(index) + ) + for index in xrange(number_of_students) + ] + class TestInstructorOra2Report(SharedModuleStoreTestCase): """ diff --git a/lms/djangoapps/student_account/test/test_views.py b/lms/djangoapps/student_account/test/test_views.py index 585233cb93..59dad97124 100644 --- a/lms/djangoapps/student_account/test/test_views.py +++ b/lms/djangoapps/student_account/test/test_views.py @@ -4,13 +4,13 @@ import re from unittest import skipUnless from urllib import urlencode -import json import mock import ddt from django.conf import settings -from django.core.urlresolvers import reverse from django.core import mail +from django.core.files.uploadedfile import SimpleUploadedFile +from django.core.urlresolvers import reverse from django.contrib import messages from django.contrib.messages.middleware import MessageMiddleware from django.test import TestCase @@ -214,9 +214,15 @@ class StudentAccountLoginAndRegistrationTest(ThirdPartyAuthTestMixin, UrlResetMi @mock.patch.dict(settings.FEATURES, {'EMBARGO': True}) def setUp(self): super(StudentAccountLoginAndRegistrationTest, self).setUp('embargo') - # For these tests, two third party auth providers are enabled by default: + + # For these tests, three third party auth providers are enabled by default: self.configure_google_provider(enabled=True) self.configure_facebook_provider(enabled=True) + self.configure_dummy_provider( + enabled=True, + icon_class='', + icon_image=SimpleUploadedFile('icon.svg', ''), + ) @ddt.data( ("signin_user", "login"), @@ -290,6 +296,8 @@ class StudentAccountLoginAndRegistrationTest(ThirdPartyAuthTestMixin, UrlResetMi ("register_user", "google-oauth2", "Google"), ("signin_user", "facebook", "Facebook"), ("register_user", "facebook", "Facebook"), + ("signin_user", "dummy", "Dummy"), + ("register_user", "dummy", "Dummy"), ) @ddt.unpack def test_third_party_auth(self, url_name, current_backend, current_provider): @@ -313,10 +321,19 @@ class StudentAccountLoginAndRegistrationTest(ThirdPartyAuthTestMixin, UrlResetMi # This relies on the THIRD_PARTY_AUTH configuration in the test settings expected_providers = [ + { + "id": "oa2-dummy", + "name": "Dummy", + "iconClass": None, + "iconImage": settings.MEDIA_URL + "icon.svg", + "loginUrl": self._third_party_login_url("dummy", "login", params), + "registerUrl": self._third_party_login_url("dummy", "register", params) + }, { "id": "oa2-facebook", "name": "Facebook", "iconClass": "fa-facebook", + "iconImage": None, "loginUrl": self._third_party_login_url("facebook", "login", params), "registerUrl": self._third_party_login_url("facebook", "register", params) }, @@ -324,9 +341,10 @@ class StudentAccountLoginAndRegistrationTest(ThirdPartyAuthTestMixin, UrlResetMi "id": "oa2-google-oauth2", "name": "Google", "iconClass": "fa-google-plus", + "iconImage": None, "loginUrl": self._third_party_login_url("google-oauth2", "login", params), "registerUrl": self._third_party_login_url("google-oauth2", "register", params) - } + }, ] self._assert_third_party_auth_data(response, current_backend, current_provider, expected_providers) diff --git a/lms/djangoapps/student_account/views.py b/lms/djangoapps/student_account/views.py index 3db78369e2..427c717d40 100644 --- a/lms/djangoapps/student_account/views.py +++ b/lms/djangoapps/student_account/views.py @@ -198,7 +198,8 @@ def _third_party_auth_context(request, redirect_to): info = { "id": enabled.provider_id, "name": enabled.name, - "iconClass": enabled.icon_class, + "iconClass": enabled.icon_class or None, + "iconImage": enabled.icon_image.url if enabled.icon_image else None, "loginUrl": pipeline.get_login_url( enabled.provider_id, pipeline.AUTH_ENTRY_LOGIN, diff --git a/lms/envs/aws.py b/lms/envs/aws.py index 8995a09385..5096767d52 100644 --- a/lms/envs/aws.py +++ b/lms/envs/aws.py @@ -574,6 +574,7 @@ if FEATURES.get('ENABLE_THIRD_PARTY_AUTH'): 'social.backends.google.GoogleOAuth2', 'social.backends.linkedin.LinkedinOAuth2', 'social.backends.facebook.FacebookOAuth2', + 'social.backends.azuread.AzureADOAuth2', 'third_party_auth.saml.SAMLAuthBackend', 'third_party_auth.lti.LTIAuthBackend', ]) + list(AUTHENTICATION_BACKENDS) diff --git a/lms/envs/bok_choy.env.json b/lms/envs/bok_choy.env.json index a745c4c615..b73ed4f7c8 100644 --- a/lms/envs/bok_choy.env.json +++ b/lms/envs/bok_choy.env.json @@ -96,7 +96,7 @@ "LOCAL_LOGLEVEL": "INFO", "LOGGING_ENV": "sandbox", "LOG_DIR": "** OVERRIDDEN **", - "MEDIA_URL": "", + "MEDIA_URL": "/media/", "MKTG_URL_LINK_MAP": { "ABOUT": "about", "PRIVACY": "privacy", diff --git a/lms/envs/bok_choy.py b/lms/envs/bok_choy.py index d461d4bbf3..366fb3dd4c 100644 --- a/lms/envs/bok_choy.py +++ b/lms/envs/bok_choy.py @@ -67,7 +67,6 @@ STATICFILES_DIRS = ( DEFAULT_FILE_STORAGE = 'django.core.files.storage.FileSystemStorage' MEDIA_ROOT = TEST_ROOT / "uploads" -MEDIA_URL = "/static/uploads/" # Don't use compression during tests PIPELINE_JS_COMPRESSOR = None diff --git a/lms/envs/test.py b/lms/envs/test.py index 505d32958f..ba43f30574 100644 --- a/lms/envs/test.py +++ b/lms/envs/test.py @@ -265,6 +265,7 @@ AUTHENTICATION_BACKENDS = ( 'social.backends.google.GoogleOAuth2', 'social.backends.linkedin.LinkedinOAuth2', 'social.backends.facebook.FacebookOAuth2', + 'social.backends.azuread.AzureADOAuth2', 'social.backends.twitter.TwitterOAuth', 'third_party_auth.dummy.DummyBackend', 'third_party_auth.saml.SAMLAuthBackend', diff --git a/lms/static/js/certificates/factories/certificate_whitelist_factory.js b/lms/static/js/certificates/factories/certificate_whitelist_factory.js index 267c63a9f2..b008e85316 100644 --- a/lms/static/js/certificates/factories/certificate_whitelist_factory.js +++ b/lms/static/js/certificates/factories/certificate_whitelist_factory.js @@ -16,7 +16,7 @@ return function(certificate_white_list_json, generate_certificate_exceptions_url, certificate_exception_view_url, generate_bulk_certificate_exceptions_url){ - var certificateWhiteList = new CertificateWhiteListCollection(JSON.parse(certificate_white_list_json), { + var certificateWhiteList = new CertificateWhiteListCollection(certificate_white_list_json, { parse: true, canBeEmpty: true, url: certificate_exception_view_url, diff --git a/lms/static/sass/base/_variables.scss b/lms/static/sass/base/_variables.scss index 6e68f74e90..f68c6b1e36 100644 --- a/lms/static/sass/base/_variables.scss +++ b/lms/static/sass/base/_variables.scss @@ -188,6 +188,8 @@ $ui-notification-height: ($baseline*10); $twitter-blue: #55ACEE; $facebook-blue: #3B5998; $linkedin-blue: #0077B5; +$google-red: #DD4B39; +$microsoft-blue: #00BCF2; // shadows $shadow: rgba(0,0,0,0.2) !default; diff --git a/lms/static/sass/multicourse/_account.scss b/lms/static/sass/multicourse/_account.scss index 1eacb688e8..600006c1e4 100644 --- a/lms/static/sass/multicourse/_account.scss +++ b/lms/static/sass/multicourse/_account.scss @@ -524,8 +524,9 @@ margin-right: ($baseline/2); .icon { - color: inherit; + @extend %sso-icon; @include margin-right($baseline/2); + color: inherit; } &:last-child { diff --git a/lms/static/sass/views/_login-register.scss b/lms/static/sass/views/_login-register.scss index e9b5cde41d..ad7431c247 100644 --- a/lms/static/sass/views/_login-register.scss +++ b/lms/static/sass/views/_login-register.scss @@ -3,10 +3,6 @@ @import '../base/grid-settings'; @import "neat/neat"; // lib - Neat -$sm-btn-google: #dd4b39; -$sm-btn-facebook: #3b5998; -$sm-btn-linkedin: #0077b5; - .login-register { @include box-sizing(border-box); @include outer-container; @@ -356,6 +352,10 @@ $sm-btn-linkedin: #0077b5; text-transform: none; font-weight: 600; letter-spacing: normal; + + .icon { + @extend %sso-icon; + } } .login-provider { @@ -375,6 +375,7 @@ $sm-btn-linkedin: #0077b5; text-transform: none; .icon { + @extend %sso-icon; @include left(0); position: absolute; @@ -403,17 +404,17 @@ $sm-btn-linkedin: #0077b5; } &.button-oa2-google-oauth2 { - color: $sm-btn-google; + color: $google-red; .icon { - background: $sm-btn-google; + background: $google-red; } &:hover, &:focus { - background-color: $sm-btn-google; + background-color: $google-red; border: 1px solid #A5382B; - color: white; + color: $white; } &:hover { @@ -422,17 +423,17 @@ $sm-btn-linkedin: #0077b5; } &.button-oa2-facebook { - color: $sm-btn-facebook; + color: $facebook-blue; .icon { - background: $sm-btn-facebook; + background: $facebook-blue; } &:hover, &:focus { - background-color: $sm-btn-facebook; + background-color: $facebook-blue; border: 1px solid #263A62; - color: white; + color: $white; } &:hover { @@ -441,17 +442,17 @@ $sm-btn-linkedin: #0077b5; } &.button-oa2-linkedin-oauth2 { - color: $sm-btn-linkedin; + color: $linkedin-blue; .icon { - background: $sm-btn-linkedin; + background: $linkedin-blue; } &:hover, &:focus { - background-color: $sm-btn-linkedin; + background-color: $linkedin-blue; border: 1px solid #06527D; - color: white; + color: $white; } &:hover { @@ -459,6 +460,25 @@ $sm-btn-linkedin: #0077b5; } } + &.button-oa2-azuread-oauth2 { + color: darken($microsoft-blue, 20%); + + .icon { + background: $microsoft-blue; + } + + &:hover, + &:focus { + background-color: $microsoft-blue; + border: 1px solid $microsoft-blue; + color: $white; + } + + &:hover { + box-shadow: 0 2px 1px 0 darken($microsoft-blue, 10%); + } + } + } .button-secondary-login { diff --git a/lms/templates/instructor/instructor_dashboard_2/certificate-white-list.underscore b/lms/templates/instructor/instructor_dashboard_2/certificate-white-list.underscore index 581c3ecfd5..0e97cc3af9 100644 --- a/lms/templates/instructor/instructor_dashboard_2/certificate-white-list.underscore +++ b/lms/templates/instructor/instructor_dashboard_2/certificate-white-list.underscore @@ -2,7 +2,8 @@


diff --git a/lms/templates/register-form.html b/lms/templates/register-form.html index c8688cbc98..c9470bf4c3 100644 --- a/lms/templates/register-form.html +++ b/lms/templates/register-form.html @@ -26,7 +26,14 @@ from student.models import UserProfile % for enabled in provider.Registry.accepting_logins(): ## Translators: provider_name is the name of an external, third-party user authentication service (like Google or LinkedIn). - + % endfor
diff --git a/lms/templates/student_account/hinted_login.underscore b/lms/templates/student_account/hinted_login.underscore index d1cb0d8379..6b7dda7605 100644 --- a/lms/templates/student_account/hinted_login.underscore +++ b/lms/templates/student_account/hinted_login.underscore @@ -8,7 +8,11 @@

<%- _.sprintf( gettext("Would you like to sign in using your %(providerName)s credentials?"), { providerName: hintedProvider.name } ) %>

diff --git a/lms/templates/student_account/login.underscore b/lms/templates/student_account/login.underscore index 5ee369923a..c28ead2c27 100644 --- a/lms/templates/student_account/login.underscore +++ b/lms/templates/student_account/login.underscore @@ -61,7 +61,11 @@ <% _.each( context.providers, function( provider ) { if ( provider.loginUrl ) { %> diff --git a/lms/templates/student_account/register.underscore b/lms/templates/student_account/register.underscore index ddd409dbda..bbcb7f12cd 100644 --- a/lms/templates/student_account/register.underscore +++ b/lms/templates/student_account/register.underscore @@ -30,7 +30,11 @@ _.each( context.providers, function( provider) { if ( provider.registerUrl ) { %> diff --git a/lms/urls.py b/lms/urls.py index 18c5cb80c4..c8b96c99c9 100644 --- a/lms/urls.py +++ b/lms/urls.py @@ -943,6 +943,7 @@ urlpatterns = patterns(*urlpatterns) if settings.DEBUG: urlpatterns += static(settings.STATIC_URL, document_root=settings.STATIC_ROOT) + urlpatterns += static(settings.MEDIA_URL, document_root=settings.MEDIA_ROOT) urlpatterns += static( settings.PROFILE_IMAGE_BACKEND['options']['base_url'], document_root=settings.PROFILE_IMAGE_BACKEND['options']['location'] diff --git a/pavelib/bok_choy.py b/pavelib/bok_choy.py index 3c4bb319fa..af76e7512a 100644 --- a/pavelib/bok_choy.py +++ b/pavelib/bok_choy.py @@ -30,7 +30,8 @@ BOKCHOY_OPTS = [ make_option("-q", "--quiet", action="store_const", const=0, dest="verbosity"), make_option("-v", "--verbosity", action="count", dest="verbosity"), make_option("--pdb", action="store_true", help="Drop into debugger on failures or errors"), - make_option("--skip_firefox_version_validation", action='store_false', dest="validate_firefox_version") + make_option("--skip_firefox_version_validation", action='store_false', dest="validate_firefox_version"), + make_option("--save_screenshots", action='store_true', dest="save_screenshots"), ] @@ -52,6 +53,7 @@ def parse_bokchoy_opts(options): 'extra_args': getattr(options, 'extra_args', ''), 'pdb': getattr(options, 'pdb', False), 'test_dir': getattr(options, 'test_dir', 'tests'), + 'save_screenshots': getattr(options, 'save_screenshots', False), } diff --git a/pavelib/utils/test/suites/bokchoy_suite.py b/pavelib/utils/test/suites/bokchoy_suite.py index bdf8566f8c..38461a311a 100644 --- a/pavelib/utils/test/suites/bokchoy_suite.py +++ b/pavelib/utils/test/suites/bokchoy_suite.py @@ -62,6 +62,7 @@ class BokChoyTestSuite(TestSuite): self.a11y_file = Env.BOK_CHOY_A11Y_CUSTOM_RULES_FILE self.imports_dir = kwargs.get('imports_dir', None) self.coveragerc = kwargs.get('coveragerc', None) + self.save_screenshots = kwargs.get('save_screenshots', False) def __enter__(self): super(BokChoyTestSuite, self).__enter__() @@ -234,6 +235,8 @@ class BokChoyTestSuite(TestSuite): ] if self.pdb: cmd.append("--pdb") + if self.save_screenshots: + cmd.append("--with-save-baseline") cmd.append(self.extra_args) cmd = (" ").join(cmd) diff --git a/screenshots/baseline/hinted-login.png b/screenshots/baseline/hinted-login.png new file mode 100644 index 0000000000..fe581ce31a Binary files /dev/null and b/screenshots/baseline/hinted-login.png differ diff --git a/screenshots/baseline/login-providers.png b/screenshots/baseline/login-providers.png new file mode 100644 index 0000000000..70c19ca226 Binary files /dev/null and b/screenshots/baseline/login-providers.png differ diff --git a/screenshots/baseline/register-providers.png b/screenshots/baseline/register-providers.png new file mode 100644 index 0000000000..43eabefd87 Binary files /dev/null and b/screenshots/baseline/register-providers.png differ diff --git a/test_root/uploads/.gitignore b/test_root/uploads/.gitignore index 409deae7b2..8e2c34b24a 100644 --- a/test_root/uploads/.gitignore +++ b/test_root/uploads/.gitignore @@ -2,3 +2,5 @@ *.jpg *.png *.txt +*.svg +!test_icon.png diff --git a/test_root/uploads/test-icon.png b/test_root/uploads/test-icon.png new file mode 100644 index 0000000000..c1c8e813fd Binary files /dev/null and b/test_root/uploads/test-icon.png differ diff --git a/themes/stanford-style/lms/templates/register-form.html b/themes/stanford-style/lms/templates/register-form.html index b498ddf130..cf5b017968 100644 --- a/themes/stanford-style/lms/templates/register-form.html +++ b/themes/stanford-style/lms/templates/register-form.html @@ -26,7 +26,14 @@ from student.models import UserProfile % for enabled in provider.Registry.accepting_logins(): ## Translators: provider_name is the name of an external, third-party user authentication service (like Google or LinkedIn). - + % endfor