From 7e20cf2215ce0bcb49ef70f414287c137ee34047 Mon Sep 17 00:00:00 2001 From: Awais Jibran Date: Wed, 6 Jan 2021 17:30:13 +0500 Subject: [PATCH 1/3] Provide logo URL from the backend --- lms/djangoapps/branding/api.py | 9 ++++++++- lms/djangoapps/bulk_email/tasks.py | 3 ++- openedx/core/djangoapps/ace_common/template_context.py | 4 ++-- 3 files changed, 12 insertions(+), 4 deletions(-) diff --git a/lms/djangoapps/branding/api.py b/lms/djangoapps/branding/api.py index 4f4ade9abc..60f2a359ce 100644 --- a/lms/djangoapps/branding/api.py +++ b/lms/djangoapps/branding/api.py @@ -13,7 +13,6 @@ the marketing site and blog). """ - import logging import six @@ -673,3 +672,11 @@ def get_home_url(): Return Dashboard page url """ return reverse('dashboard') + + +def get_logo_url_for_email(): + """ + Returns the url for the branded logo image for embedding in email templates. + """ + default_logo_url = getattr(settings, 'DEFAULT_EMAIL_LOGO_URL') + return getattr(settings, 'LOGO_URL_PNG') or default_logo_url diff --git a/lms/djangoapps/bulk_email/tasks.py b/lms/djangoapps/bulk_email/tasks.py index 65665f2289..5639911846 100644 --- a/lms/djangoapps/bulk_email/tasks.py +++ b/lms/djangoapps/bulk_email/tasks.py @@ -41,6 +41,7 @@ from edx_django_utils.monitoring import set_code_owner_attribute from markupsafe import escape from six import text_type +from lms.djangoapps.branding.api import get_logo_url_for_email from lms.djangoapps.bulk_email.models import CourseEmail, Optout from lms.djangoapps.bulk_email.api import get_unsubscribed_link from lms.djangoapps.courseware.courses import get_course @@ -123,6 +124,7 @@ def _get_course_email_context(course): 'course_end_date': course_end_date, 'account_settings_url': '{}{}'.format(lms_root_url, reverse('account_settings')), 'email_settings_url': '{}{}'.format(lms_root_url, reverse('dashboard')), + 'logo_url': get_logo_url_for_email(), 'platform_name': configuration_helpers.get_value('PLATFORM_NAME', settings.PLATFORM_NAME), 'year': timezone.now().year, } @@ -294,7 +296,6 @@ def send_course_email(entry_id, email_id, to_list, global_email_context, subtask send_exception = None new_subtask_status = None try: - course_title = global_email_context['course_title'] start_time = time.time() new_subtask_status, send_exception = _send_course_email( entry_id, diff --git a/openedx/core/djangoapps/ace_common/template_context.py b/openedx/core/djangoapps/ace_common/template_context.py index a1339e209f..53b991611e 100644 --- a/openedx/core/djangoapps/ace_common/template_context.py +++ b/openedx/core/djangoapps/ace_common/template_context.py @@ -6,6 +6,7 @@ Context dictionary for templates that use the ace_common base template. from django.conf import settings from django.urls import NoReverseMatch, reverse +from lms.djangoapps.branding.api import get_logo_url_for_email from common.djangoapps.edxmako.shortcuts import marketing_link from openedx.core.djangoapps.theming.helpers import get_config_value_from_site_or_settings @@ -16,7 +17,6 @@ def get_base_template_context(site): """ # When on LMS and a dashboard is available, use that as the dashboard url. # Otherwise, use the home url instead. - default_logo_url = getattr(settings, 'DEFAULT_EMAIL_LOGO_URL') try: dashboard_url = reverse('dashboard') except NoReverseMatch: @@ -38,5 +38,5 @@ def get_base_template_context(site): 'CONTACT_MAILING_ADDRESS', site=site, site_config_name='contact_mailing_address'), 'social_media_urls': get_config_value_from_site_or_settings('SOCIAL_MEDIA_FOOTER_URLS', site=site), 'mobile_store_urls': get_config_value_from_site_or_settings('MOBILE_STORE_URLS', site=site), - 'logo_url': getattr(settings, 'LOGO_URL_PNG', default_logo_url), + 'logo_url': get_logo_url_for_email(), } From 159031adc9f38299dd8523878356fec33a7d0f77 Mon Sep 17 00:00:00 2001 From: Awais Jibran Date: Wed, 6 Jan 2021 20:23:16 +0500 Subject: [PATCH 2/3] quality fixes --- lms/djangoapps/bulk_email/tasks.py | 2 -- lms/djangoapps/instructor/views/api.py | 19 ++++++++++++++----- .../djangoapps/ace_common/template_context.py | 1 - 3 files changed, 14 insertions(+), 8 deletions(-) diff --git a/lms/djangoapps/bulk_email/tasks.py b/lms/djangoapps/bulk_email/tasks.py index 5639911846..05a2b4fdd6 100644 --- a/lms/djangoapps/bulk_email/tasks.py +++ b/lms/djangoapps/bulk_email/tasks.py @@ -4,7 +4,6 @@ This module contains celery task functions for handling the sending of bulk emai to a course. """ - import json import logging import random @@ -59,7 +58,6 @@ from common.djangoapps.util.string_utils import _has_non_ascii_characters log = logging.getLogger('edx.celery.task') - # Errors that an individual email is failing to be sent, and should just # be treated as a fail. SINGLE_EMAIL_FAILURE_ERRORS = ( diff --git a/lms/djangoapps/instructor/views/api.py b/lms/djangoapps/instructor/views/api.py index 0c2b227af7..4fb1f3d522 100644 --- a/lms/djangoapps/instructor/views/api.py +++ b/lms/djangoapps/instructor/views/api.py @@ -6,7 +6,6 @@ JSON views which the instructor dashboard requests. Many of these GETs may become PUTs in the future. """ - import csv import json import logging @@ -186,6 +185,7 @@ def require_post_params(*args, **kwargs): required_params = [] required_params += [(arg, None) for arg in args] required_params += [(key, kwargs[key]) for key in kwargs] + # required_params = e.g. [('action', 'enroll or unenroll'), ['emails', None]] def decorator(func): @@ -208,7 +208,9 @@ def require_post_params(*args, **kwargs): return JsonResponse(error_response_data, status=400) else: return func(*args, **kwargs) + return wrapped + return decorator @@ -221,6 +223,7 @@ def require_course_permission(permission): Assumes that request is in args[0]. Assumes that course_id is in kwargs['course_id']. """ + def decorator(func): def wrapped(*args, **kwargs): request = args[0] @@ -230,7 +233,9 @@ def require_course_permission(permission): return func(*args, **kwargs) else: return HttpResponseForbidden() + return wrapped + return decorator @@ -242,6 +247,7 @@ def require_sales_admin(func): If the user does not have privileges for this operation, this will return HttpResponseForbidden (403). """ + def wrapped(request, course_id): try: @@ -256,6 +262,7 @@ def require_sales_admin(func): return func(request, course_id) else: return HttpResponseForbidden() + return wrapped @@ -267,6 +274,7 @@ def require_finance_admin(func): If the user does not have privileges for this operation, this will return HttpResponseForbidden (403). """ + def wrapped(request, course_id): try: @@ -281,6 +289,7 @@ def require_finance_admin(func): return func(request, course_id) else: return HttpResponseForbidden() + return wrapped @@ -315,8 +324,8 @@ def register_and_enroll_students(request, course_id): # pylint: disable=too-man """ if not configuration_helpers.get_value( - 'ALLOW_AUTOMATED_SIGNUPS', - settings.FEATURES.get('ALLOW_AUTOMATED_SIGNUPS', False), + 'ALLOW_AUTOMATED_SIGNUPS', + settings.FEATURES.get('ALLOW_AUTOMATED_SIGNUPS', False), ): return HttpResponseForbidden() @@ -2959,8 +2968,8 @@ def invalidate_certificate(request, generated_certificate, certificate_invalidat :return: dict object containing updated certificate invalidation data. """ if CertificateInvalidation.get_certificate_invalidations( - generated_certificate.course_id, - generated_certificate.user, + generated_certificate.course_id, + generated_certificate.user, ): raise ValueError( _(u"Certificate of {user} has already been invalidated. Please check your spelling and retry.").format( diff --git a/openedx/core/djangoapps/ace_common/template_context.py b/openedx/core/djangoapps/ace_common/template_context.py index 53b991611e..303bfcc204 100644 --- a/openedx/core/djangoapps/ace_common/template_context.py +++ b/openedx/core/djangoapps/ace_common/template_context.py @@ -2,7 +2,6 @@ Context dictionary for templates that use the ace_common base template. """ - from django.conf import settings from django.urls import NoReverseMatch, reverse From 5a04fb66ff12ba9a9b9f29a1435c7bec25fdfb00 Mon Sep 17 00:00:00 2001 From: Awais Jibran Date: Wed, 6 Jan 2021 22:08:50 +0500 Subject: [PATCH 3/3] quality fixes --- lms/djangoapps/branding/api.py | 4 ++-- lms/djangoapps/instructor/views/api.py | 10 ---------- 2 files changed, 2 insertions(+), 12 deletions(-) diff --git a/lms/djangoapps/branding/api.py b/lms/djangoapps/branding/api.py index 60f2a359ce..01e240d5dc 100644 --- a/lms/djangoapps/branding/api.py +++ b/lms/djangoapps/branding/api.py @@ -678,5 +678,5 @@ def get_logo_url_for_email(): """ Returns the url for the branded logo image for embedding in email templates. """ - default_logo_url = getattr(settings, 'DEFAULT_EMAIL_LOGO_URL') - return getattr(settings, 'LOGO_URL_PNG') or default_logo_url + default_logo_url = getattr(settings, 'DEFAULT_EMAIL_LOGO_URL', None) + return getattr(settings, 'LOGO_URL_PNG', None) or default_logo_url diff --git a/lms/djangoapps/instructor/views/api.py b/lms/djangoapps/instructor/views/api.py index 4fb1f3d522..3cf21cd0d8 100644 --- a/lms/djangoapps/instructor/views/api.py +++ b/lms/djangoapps/instructor/views/api.py @@ -185,7 +185,6 @@ def require_post_params(*args, **kwargs): required_params = [] required_params += [(arg, None) for arg in args] required_params += [(key, kwargs[key]) for key in kwargs] - # required_params = e.g. [('action', 'enroll or unenroll'), ['emails', None]] def decorator(func): @@ -208,9 +207,7 @@ def require_post_params(*args, **kwargs): return JsonResponse(error_response_data, status=400) else: return func(*args, **kwargs) - return wrapped - return decorator @@ -223,7 +220,6 @@ def require_course_permission(permission): Assumes that request is in args[0]. Assumes that course_id is in kwargs['course_id']. """ - def decorator(func): def wrapped(*args, **kwargs): request = args[0] @@ -233,9 +229,7 @@ def require_course_permission(permission): return func(*args, **kwargs) else: return HttpResponseForbidden() - return wrapped - return decorator @@ -247,7 +241,6 @@ def require_sales_admin(func): If the user does not have privileges for this operation, this will return HttpResponseForbidden (403). """ - def wrapped(request, course_id): try: @@ -262,7 +255,6 @@ def require_sales_admin(func): return func(request, course_id) else: return HttpResponseForbidden() - return wrapped @@ -274,7 +266,6 @@ def require_finance_admin(func): If the user does not have privileges for this operation, this will return HttpResponseForbidden (403). """ - def wrapped(request, course_id): try: @@ -289,7 +280,6 @@ def require_finance_admin(func): return func(request, course_id) else: return HttpResponseForbidden() - return wrapped