From a9476ea50eeb1c3b93bd976403f4e9492961619a Mon Sep 17 00:00:00 2001 From: Daniel Clemente Laboreo Date: Thu, 23 May 2019 16:33:30 +0300 Subject: [PATCH 01/20] Add last_login and date_joined to the student profile export --- lms/djangoapps/instructor/views/api.py | 3 +++ lms/djangoapps/instructor_analytics/basic.py | 3 ++- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/lms/djangoapps/instructor/views/api.py b/lms/djangoapps/instructor/views/api.py index 1a8aed27ad..3cf7e7e1b4 100644 --- a/lms/djangoapps/instructor/views/api.py +++ b/lms/djangoapps/instructor/views/api.py @@ -1317,6 +1317,7 @@ def get_students_features(request, course_id, csv=False): # pylint: disable=red 'id', 'username', 'name', 'email', 'language', 'location', 'year_of_birth', 'gender', 'level_of_education', 'mailing_address', 'goals', 'enrollment_mode', 'verification_status', + 'last_login', 'date_joined', ] # Provide human-friendly and translatable names for these features. These names @@ -1336,6 +1337,8 @@ def get_students_features(request, course_id, csv=False): # pylint: disable=red 'goals': _('Goals'), 'enrollment_mode': _('Enrollment Mode'), 'verification_status': _('Verification Status'), + 'last_login': _('Last Login'), + 'date_joined': _('Date Joined'), } if is_course_cohorted(course.id): diff --git a/lms/djangoapps/instructor_analytics/basic.py b/lms/djangoapps/instructor_analytics/basic.py index d45d792446..9f48160918 100644 --- a/lms/djangoapps/instructor_analytics/basic.py +++ b/lms/djangoapps/instructor_analytics/basic.py @@ -39,7 +39,8 @@ from student.models import CourseEnrollment, CourseEnrollmentAllowed log = logging.getLogger(__name__) -STUDENT_FEATURES = ('id', 'username', 'first_name', 'last_name', 'is_staff', 'email') +STUDENT_FEATURES = ('id', 'username', 'first_name', 'last_name', 'is_staff', 'email', + 'date_joined', 'last_login') PROFILE_FEATURES = ('name', 'language', 'location', 'year_of_birth', 'gender', 'level_of_education', 'mailing_address', 'goals', 'meta', 'city', 'country') From 7f623d15d803de5ccba5e5ee3a5a1b14abbbac94 Mon Sep 17 00:00:00 2001 From: Matthew Piatetsky Date: Tue, 3 Dec 2019 15:07:09 -0500 Subject: [PATCH 02/20] get optimizely from cloudflare --- lms/templates/widgets/optimizely.html | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lms/templates/widgets/optimizely.html b/lms/templates/widgets/optimizely.html index 28986cf32a..614600b2a5 100644 --- a/lms/templates/widgets/optimizely.html +++ b/lms/templates/widgets/optimizely.html @@ -1,5 +1,5 @@ <%page expression_filter="h"/> % if settings.OPTIMIZELY_PROJECT_ID and not disable_optimizely and not is_from_mobile_app: - + % endif From 9e4706e7bb40e76c374056f8c9c44040a568b772 Mon Sep 17 00:00:00 2001 From: Robert Raposa Date: Thu, 5 Dec 2019 16:42:26 -0500 Subject: [PATCH 03/20] remove UPDATE_LOGIN_USER_ERROR_STATUS_CODE toggle The toggle UPDATE_LOGIN_USER_ERROR_STATUS_CODE was added to roll out a breaking change for `login_user` auth errors to return a 400 rather than a 200. This toggle was enabled in Production on 12/5/2019 with seemingly no adverse affects. ARCH-1253 --- .../third_party_auth/tests/specs/base.py | 2 +- .../djangoapps/user_authn/config/waffle.py | 15 --------- .../core/djangoapps/user_authn/views/login.py | 11 ++----- .../user_authn/views/tests/test_login.py | 31 +++++++------------ 4 files changed, 14 insertions(+), 45 deletions(-) diff --git a/common/djangoapps/third_party_auth/tests/specs/base.py b/common/djangoapps/third_party_auth/tests/specs/base.py index 08dfc6d175..f29099308d 100644 --- a/common/djangoapps/third_party_auth/tests/specs/base.py +++ b/common/djangoapps/third_party_auth/tests/specs/base.py @@ -123,7 +123,7 @@ class HelperMixin(object): def assert_json_failure_response_is_inactive_account(self, response): """Asserts failure on /login for inactive account looks right.""" - self.assertEqual(200, response.status_code) # Yes, it's a 200 even though it's a failure. + self.assertEqual(400, response.status_code) payload = json.loads(response.content.decode('utf-8')) self.assertFalse(payload.get('success')) self.assertIn('In order to sign in, you need to activate your account.', payload.get('value')) diff --git a/openedx/core/djangoapps/user_authn/config/waffle.py b/openedx/core/djangoapps/user_authn/config/waffle.py index 75cb4a801e..ece17d749b 100644 --- a/openedx/core/djangoapps/user_authn/config/waffle.py +++ b/openedx/core/djangoapps/user_authn/config/waffle.py @@ -22,18 +22,3 @@ _WAFFLE_SWITCH_NAMESPACE = WaffleSwitchNamespace(name=_WAFFLE_NAMESPACE, log_pre ENABLE_LOGIN_USING_THIRDPARTY_AUTH_ONLY = WaffleSwitch( _WAFFLE_SWITCH_NAMESPACE, 'enable_login_using_thirdparty_auth_only' ) - -# .. toggle_name: user_authn.update_login_user_error_status_code -# .. toggle_implementation: WaffleSwitch -# .. toggle_default: False -# .. toggle_description: Changes auth failures (non-SSO) from 200 to 400. -# .. toggle_category: authn -# .. toggle_use_cases: incremental_release -# .. toggle_creation_date: 2019-11-21 -# .. toggle_expiration_date: 2020-01-31 -# .. toggle_warnings: Causes backward incompatible change. Document before removing. -# .. toggle_tickets: ARCH-1253 -# .. toggle_status: supported -UPDATE_LOGIN_USER_ERROR_STATUS_CODE = WaffleSwitch( - _WAFFLE_SWITCH_NAMESPACE, 'update_login_user_error_status_code' -) diff --git a/openedx/core/djangoapps/user_authn/views/login.py b/openedx/core/djangoapps/user_authn/views/login.py index 451d88cdbb..268b5f5365 100644 --- a/openedx/core/djangoapps/user_authn/views/login.py +++ b/openedx/core/djangoapps/user_authn/views/login.py @@ -34,10 +34,7 @@ from openedx.core.djangoapps.user_authn.cookies import refresh_jwt_cookies, set_ from openedx.core.djangoapps.user_authn.exceptions import AuthFailedError from openedx.core.djangoapps.util.user_messages import PageLevelMessages from openedx.core.djangoapps.user_authn.views.password_reset import send_password_reset_email_for_user -from openedx.core.djangoapps.user_authn.config.waffle import ( - ENABLE_LOGIN_USING_THIRDPARTY_AUTH_ONLY, - UPDATE_LOGIN_USER_ERROR_STATUS_CODE -) +from openedx.core.djangoapps.user_authn.config.waffle import ENABLE_LOGIN_USING_THIRDPARTY_AUTH_ONLY from openedx.core.djangolib.markup import HTML, Text from openedx.core.lib.api.view_utils import require_post_params from student.models import LoginFailures, AllowedAuthUser, UserProfile @@ -406,11 +403,7 @@ def login_user(request): return response except AuthFailedError as error: log.exception(error.get_response()) - # original code returned a 200 status code with status=False for errors. This flag - # is used for rolling out a transition to using a 400 status code for errors, which - # is a breaking-change, but will hopefully be a tolerable breaking-change. - status = 400 if UPDATE_LOGIN_USER_ERROR_STATUS_CODE.is_enabled() else 200 - response = JsonResponse(error.get_response(), status=status) + response = JsonResponse(error.get_response(), status=400) set_custom_metric('login_user_auth_failed_error', True) set_custom_metric('login_user_response_status', response.status_code) return response diff --git a/openedx/core/djangoapps/user_authn/views/tests/test_login.py b/openedx/core/djangoapps/user_authn/views/tests/test_login.py index 967f121c4d..49e6d84cda 100644 --- a/openedx/core/djangoapps/user_authn/views/tests/test_login.py +++ b/openedx/core/djangoapps/user_authn/views/tests/test_login.py @@ -32,7 +32,6 @@ from openedx.core.djangoapps.user_authn.cookies import jwt_cookies from openedx.core.djangoapps.user_authn.views.login import ( shim_student_view, AllowedAuthUser, - UPDATE_LOGIN_USER_ERROR_STATUS_CODE, ENABLE_LOGIN_USING_THIRDPARTY_AUTH_ONLY ) from openedx.core.djangoapps.user_authn.tests.utils import setup_login_oauth_client @@ -84,12 +83,10 @@ class LoginTest(SiteMixin, CacheIsolationTestCase): self._assert_audit_log(mock_audit_log, 'info', [u'Login success', self.user_email]) @patch.dict("django.conf.settings.FEATURES", {'SQUELCH_PII_IN_LOGS': True}) - @ddt.data(True, False) - def test_login_success_no_pii(self, is_error_status_code_enabled): - with UPDATE_LOGIN_USER_ERROR_STATUS_CODE.override(is_error_status_code_enabled): - response, mock_audit_log = self._login_response( - self.user_email, self.password, patched_audit_log='student.models.AUDIT_LOG' - ) + def test_login_success_no_pii(self): + response, mock_audit_log = self._login_response( + self.user_email, self.password, patched_audit_log='student.models.AUDIT_LOG' + ) self._assert_response(response, success=True) self._assert_audit_log(mock_audit_log, 'info', [u'Login success']) self._assert_not_in_audit_log(mock_audit_log, 'info', [self.user_email]) @@ -118,20 +115,14 @@ class LoginTest(SiteMixin, CacheIsolationTestCase): self.user.refresh_from_db() assert old_last_login == self.user.last_login - @ddt.data( - (True, 400), - (False, 200), - ) - @ddt.unpack - def test_login_fail_no_user_exists(self, is_error_status_code_enabled, expected_status_code): + def test_login_fail_no_user_exists(self): nonexistent_email = u'not_a_user@edx.org' - with UPDATE_LOGIN_USER_ERROR_STATUS_CODE.override(is_error_status_code_enabled): - response, mock_audit_log = self._login_response( - nonexistent_email, - self.password, - ) + response, mock_audit_log = self._login_response( + nonexistent_email, + self.password, + ) self._assert_response( - response, success=False, value=self.LOGIN_FAILED_WARNING, status_code=expected_status_code + response, success=False, value=self.LOGIN_FAILED_WARNING, status_code=400 ) self._assert_audit_log(mock_audit_log, 'warning', [u'Login failed', u'Unknown user email', nonexistent_email]) @@ -519,7 +510,7 @@ class LoginTest(SiteMixin, CacheIsolationTestCase): If value is provided, assert that the response contained that value for 'value' in the JSON dict. """ - expected_status_code = status_code or 200 + expected_status_code = status_code or (400 if success is False else 200) self.assertEqual(response.status_code, expected_status_code) try: From e7d5b0e6b113c9c3272b8fee1011a39fd1827f6a Mon Sep 17 00:00:00 2001 From: edX requirements bot Date: Mon, 9 Dec 2019 05:49:49 -0500 Subject: [PATCH 04/20] Updating Python Requirements --- requirements/edx/development.txt | 4 ++-- requirements/edx/testing.txt | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/requirements/edx/development.txt b/requirements/edx/development.txt index a21ac12c02..f68f340a62 100644 --- a/requirements/edx/development.txt +++ b/requirements/edx/development.txt @@ -55,7 +55,7 @@ git+https://github.com/edx/openedx-chem.git@ff4e3a03d3c7610e47a9af08eb648d8aabe2 click-log==0.3.2 click==7.0 code-annotations==0.3.2 -colorama==0.4.1 +colorama==0.4.3 configparser==4.0.2 contextlib2==0.6.0.post1 cookies==2.2.1 @@ -238,7 +238,7 @@ polib==1.1.0 psutil==1.2.1 py2neo==3.1.2 py==1.8.0 -pyaml==19.4.1 +pyaml==19.12.0 pycodestyle==2.5.0 pycontracts==1.7.1 pycountry==19.8.18 diff --git a/requirements/edx/testing.txt b/requirements/edx/testing.txt index 361eb7957b..d474a71397 100644 --- a/requirements/edx/testing.txt +++ b/requirements/edx/testing.txt @@ -54,7 +54,7 @@ git+https://github.com/edx/openedx-chem.git@ff4e3a03d3c7610e47a9af08eb648d8aabe2 click-log==0.3.2 # via edx-lint click==7.0 code-annotations==0.3.2 -colorama==0.4.1 # via radon +colorama==0.4.3 # via radon configparser==4.0.2 contextlib2==0.6.0.post1 cookies==2.2.1 # via moto @@ -228,7 +228,7 @@ polib==1.1.0 psutil==1.2.1 py2neo==3.1.2 py==1.8.0 # via pytest, tox -pyaml==19.4.1 # via moto +pyaml==19.12.0 # via moto pycodestyle==2.5.0 pycontracts==1.7.1 pycountry==19.8.18 From 00653433a5370378589ce3a4da4e0cd6825247dc Mon Sep 17 00:00:00 2001 From: Taranjeet Singh Date: Thu, 13 Sep 2018 12:49:47 +0530 Subject: [PATCH 05/20] Adds optional "unsubscribe" link and api support to let users opt out of email updates. Scheduled emails show "unsubscribe" link if waffle switch `schedules.course_update_show_unsubscribe` is enabled, and settings.ACE_ENABLED_POLICIES respects `bulk_email_optout`. API endpoint allows GET/POST requests, which: * GET asks for confirmation of opt-out * POST accepts "unsubscribe" or "cancel", where "unsubscribe" creates the Optout entry, and "cancel" does nothing. Fixes flaky tests: * The resolvers handle users in "bins", which are groups that depend on the user ID. * The test user ID varies depending on the test order. * This change ensures that the bin requested matches the user for the test. --- lms/djangoapps/bulk_email/tests/test_views.py | 82 ++++++++++++++++ lms/djangoapps/bulk_email/urls.py | 18 ++++ lms/djangoapps/bulk_email/views.py | 72 ++++++++++++++ lms/templates/bulk_email/unsubscribe.html | 48 +++++++++ lms/urls.py | 4 + .../ace_common/edx_ace/common/base_body.html | 7 ++ openedx/core/djangoapps/schedules/config.py | 8 +- .../core/djangoapps/schedules/resolvers.py | 20 +++- .../schedules/tests/test_resolvers.py | 97 +++++++++++++++++-- openedx/core/djangolib/testing/utils.py | 31 +++--- 10 files changed, 363 insertions(+), 24 deletions(-) create mode 100644 lms/djangoapps/bulk_email/tests/test_views.py create mode 100644 lms/djangoapps/bulk_email/urls.py create mode 100644 lms/djangoapps/bulk_email/views.py create mode 100644 lms/templates/bulk_email/unsubscribe.html diff --git a/lms/djangoapps/bulk_email/tests/test_views.py b/lms/djangoapps/bulk_email/tests/test_views.py new file mode 100644 index 0000000000..54151b8b98 --- /dev/null +++ b/lms/djangoapps/bulk_email/tests/test_views.py @@ -0,0 +1,82 @@ +# -*- coding: utf-8 -*- +""" +Test the bulk email opt out view. +""" +from six import text_type + +import ddt +from django.http import Http404 +from django.test.client import RequestFactory +from django.test.utils import override_settings +from django.urls import reverse + +from bulk_email.models import Optout +from bulk_email.views import opt_out_email_updates +from lms.djangoapps.discussion.notification_prefs.views import UsernameCipher +from openedx.core.lib.tests import attr +from student.tests.factories import UserFactory +from xmodule.modulestore.tests.factories import CourseFactory +from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase + + +@attr(shard=1) +@ddt.ddt +@override_settings(SECRET_KEY="test secret key") +class OptOutEmailUpdatesViewTest(ModuleStoreTestCase): + """ + Check the opt out email functionality. + """ + def setUp(self): + super(OptOutEmailUpdatesViewTest, self).setUp() + self.user = UserFactory.create(username="testuser1") + self.token = UsernameCipher.encrypt('testuser1') + self.request_factory = RequestFactory() + self.course = CourseFactory.create(run='testcourse1', display_name='Test Course Title') + self.url = reverse('bulk_email_opt_out', args=[self.token, text_type(self.course.id)]) + + # Ensure we start with no opt-out records + self.assertEqual(Optout.objects.count(), 0) + + def test_opt_out_email_confirm(self): + """ + Ensure that the default GET view asks for confirmation. + """ + response = self.client.get(self.url) + self.assertContains(response, "Do you want to unsubscribe from emails for Test Course Title?") + self.assertEqual(Optout.objects.count(), 0) + + def test_opt_out_email_unsubscribe(self): + """ + Ensure that the POSTing "confirm" creates the opt-out record. + """ + response = self.client.post(self.url, {'submit': 'confirm'}) + self.assertContains(response, "You have been unsubscribed from emails for Test Course Title.") + self.assertEqual(Optout.objects.count(), 1) + + def test_opt_out_email_cancel(self): + """ + Ensure that the POSTing "cancel" does not create the opt-out record + """ + response = self.client.post(self.url, {'submit': 'cancel'}) + self.assertContains(response, "You have not been unsubscribed from emails for Test Course Title.") + self.assertEqual(Optout.objects.count(), 0) + + @ddt.data( + ("ZOMG INVALID BASE64 CHARS!!!", "base64url", False), + ("Non-ASCII\xff", "base64url", False), + ("D6L8Q01ztywqnr3coMOlq0C3DG05686lXX_1ArEd0ok", "base64url", False), + ("AAAAAAAAAAA=", "initialization_vector", False), + ("nMXVK7PdSlKPOovci-M7iqS09Ux8VoCNDJixLBmj", "aes", False), + ("AAAAAAAAAAAAAAAAAAAAAMoazRI7ePLjEWXN1N7keLw=", "padding", False), + ("AAAAAAAAAAAAAAAAAAAAACpyUxTGIrUjnpuUsNi7mAY=", "username", False), + ("_KHGdCAUIToc4iaRGy7K57mNZiiXxO61qfKT08ExlY8=", "course", 'course-v1:testcourse'), + ) + @ddt.unpack + def test_unsubscribe_invalid_token(self, token, message, course): + """ + Make sure that view returns 404 in case token is not valid + """ + request = self.request_factory.get("dummy") + with self.assertRaises(Http404) as err: + opt_out_email_updates(request, token, course) + self.assertIn(message, err) diff --git a/lms/djangoapps/bulk_email/urls.py b/lms/djangoapps/bulk_email/urls.py new file mode 100644 index 0000000000..9beea793e1 --- /dev/null +++ b/lms/djangoapps/bulk_email/urls.py @@ -0,0 +1,18 @@ +""" +URLs for bulk_email app +""" + +from django.conf import settings +from django.conf.urls import url + +from bulk_email import views + +urlpatterns = [ + url( + r'^email/optout/(?P[a-zA-Z0-9-_=]+)/{}/$'.format( + settings.COURSE_ID_PATTERN, + ), + views.opt_out_email_updates, + name='bulk_email_opt_out', + ), +] diff --git a/lms/djangoapps/bulk_email/views.py b/lms/djangoapps/bulk_email/views.py new file mode 100644 index 0000000000..5e59d63c7a --- /dev/null +++ b/lms/djangoapps/bulk_email/views.py @@ -0,0 +1,72 @@ +""" +Views to support bulk email functionalities like opt-out. +""" + +from __future__ import division + +import logging + +from six import text_type + +from django.contrib.auth.models import User +from django.http import Http404 + +from bulk_email.models import Optout +from courseware.courses import get_course_by_id +from edxmako.shortcuts import render_to_response +from lms.djangoapps.discussion.notification_prefs.views import ( + UsernameCipher, + UsernameDecryptionException, +) + +from opaque_keys import InvalidKeyError +from opaque_keys.edx.keys import CourseKey + + +log = logging.getLogger(__name__) + + +def opt_out_email_updates(request, token, course_id): + """ + A view that let users opt out of any email updates. + + This meant is meant to be the target of an opt-out link or button. + The `token` parameter must decrypt to a valid username. + The `course_id` is the string course key of any course. + + Raises a 404 if there are any errors parsing the input. + """ + try: + username = UsernameCipher().decrypt(token.encode()) + user = User.objects.get(username=username) + course_key = CourseKey.from_string(course_id) + course = get_course_by_id(course_key, depth=0) + except UnicodeDecodeError: + raise Http404("base64url") + except UsernameDecryptionException as exn: + raise Http404(text_type(exn)) + except User.DoesNotExist: + raise Http404("username") + except InvalidKeyError: + raise Http404("course") + + context = { + 'course': course, + 'cancelled': False, + 'confirmed': False, + } + + if request.method == 'POST': + if request.POST.get('submit') == 'confirm': + Optout.objects.get_or_create(user=user, course_id=course.id) + log.info( + u"User %s (%s) opted out of receiving emails from course %s", + user.username, + user.email, + course_id, + ) + context['confirmed'] = True + else: + context['cancelled'] = True + + return render_to_response('bulk_email/unsubscribe.html', context) diff --git a/lms/templates/bulk_email/unsubscribe.html b/lms/templates/bulk_email/unsubscribe.html new file mode 100644 index 0000000000..270f8dc604 --- /dev/null +++ b/lms/templates/bulk_email/unsubscribe.html @@ -0,0 +1,48 @@ +<%page expression_filter="h" /> +<%inherit file="../main.html" /> +<%! + from openedx.core.djangolib.markup import Text + from django.utils.translation import ugettext as _ +%> +<%def name="header()"> +%if confirmed: + ${Text(_("Unsubscribe Successful"))} +%elif cancelled: + ${Text(_("Unsubscribe Cancelled"))} +%else: + ${Text(_("Confirm Unsubscribe"))} +%endif + + +<%block name="pagetitle">${header()} +
+ +
+

+ <%block name="pageheader">${header()} +

+

+ <%block name="pagecontent"> + %if confirmed: + ${Text(_("You have been unsubscribed from emails for {course}.")).format( + course=course.display_name_with_default + )} + %elif cancelled: + ${Text(_("You have not been unsubscribed from emails for {course}.")).format( + course=course.display_name_with_default + )} + %else: + ${Text(_("Do you want to unsubscribe from emails for {course}?")).format( + course=course.display_name_with_default + )} +

+

+ + + +
+ %endif + +

+
+
diff --git a/lms/urls.py b/lms/urls.py index 840956af5b..bbf60f4565 100644 --- a/lms/urls.py +++ b/lms/urls.py @@ -722,6 +722,10 @@ if settings.FEATURES.get('ENABLE_DISCUSSION_SERVICE'): ), ] +urlpatterns += [ + url(r'^bulk_email/', include('bulk_email.urls')), +] + urlpatterns += [ url( r'^courses/{}/tab/(?P[^/]+)/$'.format( diff --git a/openedx/core/djangoapps/ace_common/templates/ace_common/edx_ace/common/base_body.html b/openedx/core/djangoapps/ace_common/templates/ace_common/edx_ace/common/base_body.html index 044514b40d..7130554dcf 100644 --- a/openedx/core/djangoapps/ace_common/templates/ace_common/edx_ace/common/base_body.html +++ b/openedx/core/djangoapps/ace_common/templates/ace_common/edx_ace/common/base_body.html @@ -177,6 +177,13 @@ {{ contact_mailing_address }} + {% if unsubscribe_url %} + + + {% trans "Unsubscribe from these emails." as tmsg %}{{ tmsg | force_escape }} + + + {% endif %} diff --git a/openedx/core/djangoapps/schedules/config.py b/openedx/core/djangoapps/schedules/config.py index ae110465d8..b9e4e73a32 100644 --- a/openedx/core/djangoapps/schedules/config.py +++ b/openedx/core/djangoapps/schedules/config.py @@ -3,9 +3,13 @@ Contains configuration for schedules app """ from __future__ import absolute_import -from openedx.core.djangoapps.waffle_utils import CourseWaffleFlag, WaffleFlag, WaffleFlagNamespace +from openedx.core.djangoapps.waffle_utils import ( + WaffleFlagNamespace, CourseWaffleFlag, WaffleFlag, + WaffleSwitch, WaffleSwitchNamespace, +) WAFFLE_FLAG_NAMESPACE = WaffleFlagNamespace(name=u'schedules') +WAFFLE_SWITCH_NAMESPACE = WaffleSwitchNamespace(name=u'schedules') CREATE_SCHEDULE_WAFFLE_FLAG = CourseWaffleFlag( waffle_namespace=WAFFLE_FLAG_NAMESPACE, @@ -20,3 +24,5 @@ COURSE_UPDATE_WAFFLE_FLAG = CourseWaffleFlag( ) DEBUG_MESSAGE_WAFFLE_FLAG = WaffleFlag(WAFFLE_FLAG_NAMESPACE, u'enable_debugging') + +COURSE_UPDATE_SHOW_UNSUBSCRIBE_WAFFLE_SWITCH = WaffleSwitch(WAFFLE_SWITCH_NAMESPACE, u'course_update_show_unsubscribe') diff --git a/openedx/core/djangoapps/schedules/resolvers.py b/openedx/core/djangoapps/schedules/resolvers.py index 9011382635..51d2a139de 100644 --- a/openedx/core/djangoapps/schedules/resolvers.py +++ b/openedx/core/djangoapps/schedules/resolvers.py @@ -15,7 +15,9 @@ from edx_ace.recipient_resolver import RecipientResolver from edx_django_utils.monitoring import function_trace, set_custom_metric from lms.djangoapps.courseware.date_summary import verified_upgrade_deadline_link, verified_upgrade_link_is_valid +from lms.djangoapps.discussion.notification_prefs.views import UsernameCipher from openedx.core.djangoapps.ace_common.template_context import get_base_template_context +from openedx.core.djangoapps.schedules.config import COURSE_UPDATE_SHOW_UNSUBSCRIBE_WAFFLE_SWITCH from openedx.core.djangoapps.schedules.content_highlights import get_week_highlights from openedx.core.djangoapps.schedules.exceptions import CourseUpdateDoesNotExist from openedx.core.djangoapps.schedules.models import Schedule, ScheduleExperience @@ -91,6 +93,13 @@ class BinnedSchedulesBaseResolver(PrefixedDebugLoggerMixin, RecipientResolver): with function_trace('enqueue_send_task'): self.async_send_task.apply_async((self.site.id, str(msg)), retry=False) + @classmethod + def bin_num_for_user_id(cls, user_id): + """ + Returns the bin number used for the given (numeric) user ID. + """ + return user_id % cls.num_bins + def get_schedules_with_target_date_by_bin_and_orgs( self, order_by='enrollment__user__id' ): @@ -109,7 +118,7 @@ class BinnedSchedulesBaseResolver(PrefixedDebugLoggerMixin, RecipientResolver): courseenrollment__is_active=True, **schedule_day_equals_target_day_filter ).annotate( - id_mod=F('id') % self.num_bins + id_mod=self.bin_num_for_user_id(F('id')) ).filter( id_mod=self.bin_num ) @@ -363,6 +372,14 @@ class CourseUpdateResolver(BinnedSchedulesBaseResolver): ) # continue to the next schedule, don't yield an email for this one else: + unsubscribe_url = None + if (COURSE_UPDATE_SHOW_UNSUBSCRIBE_WAFFLE_SWITCH.is_enabled() and + 'bulk_email_optout' in settings.ACE_ENABLED_POLICIES): + unsubscribe_url = reverse('bulk_email_opt_out', kwargs={ + 'token': UsernameCipher.encrypt(user.username), + 'course_id': str(enrollment.course_id), + }) + template_context.update({ 'course_name': schedule.enrollment.course.display_name, 'course_url': _get_trackable_course_home_url(enrollment.course_id), @@ -372,6 +389,7 @@ class CourseUpdateResolver(BinnedSchedulesBaseResolver): # This is used by the bulk email optout policy 'course_ids': [str(enrollment.course_id)], + 'unsubscribe_url': unsubscribe_url, }) template_context.update(_get_upsell_information_for_schedule(user, schedule)) diff --git a/openedx/core/djangoapps/schedules/tests/test_resolvers.py b/openedx/core/djangoapps/schedules/tests/test_resolvers.py index ed0687347f..89f9a0b7b4 100644 --- a/openedx/core/djangoapps/schedules/tests/test_resolvers.py +++ b/openedx/core/djangoapps/schedules/tests/test_resolvers.py @@ -8,25 +8,47 @@ from unittest import skipUnless import ddt from django.conf import settings -from mock import Mock +from django.test import TestCase +from django.test.utils import override_settings +from mock import Mock, patch +from waffle.testutils import override_switch -from openedx.core.djangoapps.schedules.resolvers import BinnedSchedulesBaseResolver +from openedx.core.djangoapps.schedules.config import COURSE_UPDATE_WAFFLE_FLAG +from openedx.core.djangoapps.schedules.resolvers import ( + BinnedSchedulesBaseResolver, + CourseUpdateResolver, +) from openedx.core.djangoapps.schedules.tests.factories import ScheduleConfigFactory from openedx.core.djangoapps.site_configuration.tests.factories import SiteConfigurationFactory, SiteFactory -from openedx.core.djangolib.testing.utils import CacheIsolationTestCase, skip_unless_lms +from openedx.core.djangoapps.waffle_utils.testutils import override_waffle_flag +from openedx.core.djangolib.testing.utils import CacheIsolationMixin, skip_unless_lms +from student.tests.factories import CourseEnrollmentFactory +from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase +from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory + + +class SchedulesResolverTestMixin(CacheIsolationMixin): + """ + Base class for the resolver tests. + """ + def setUp(self): + super(SchedulesResolverTestMixin, self).setUp() + self.site = SiteFactory.create() + self.site_config = SiteConfigurationFactory(site=self.site) + self.schedule_config = ScheduleConfigFactory.create(site=self.site) @ddt.ddt @skip_unless_lms @skipUnless('openedx.core.djangoapps.schedules.apps.SchedulesConfig' in settings.INSTALLED_APPS, "Can't test schedules if the app isn't installed") -class TestBinnedSchedulesBaseResolver(CacheIsolationTestCase): +class TestBinnedSchedulesBaseResolver(SchedulesResolverTestMixin, TestCase): + """ + Tests the BinnedSchedulesBaseResolver. + """ def setUp(self): super(TestBinnedSchedulesBaseResolver, self).setUp() - self.site = SiteFactory.create() - self.site_config = SiteConfigurationFactory(site=self.site) - self.schedule_config = ScheduleConfigFactory.create(site=self.site) self.resolver = BinnedSchedulesBaseResolver( async_send_task=Mock(name='async_send_task'), site=self.site, @@ -72,3 +94,64 @@ class TestBinnedSchedulesBaseResolver(CacheIsolationTestCase): result = self.resolver.filter_by_org(mock_query) mock_query.exclude.assert_called_once_with(enrollment__course__org__in=expected_org_list) self.assertEqual(result, mock_query.exclude.return_value) + + +@skip_unless_lms +@skipUnless('openedx.core.djangoapps.schedules.apps.SchedulesConfig' in settings.INSTALLED_APPS, + "Can't test schedules if the app isn't installed") +class TestCourseUpdateResolver(SchedulesResolverTestMixin, ModuleStoreTestCase): + """ + Tests the CourseUpdateResolver. + """ + def setUp(self): + super(TestCourseUpdateResolver, self).setUp() + self.course = CourseFactory(highlights_enabled_for_messaging=True, self_paced=True) + with self.store.bulk_operations(self.course.id): + ItemFactory.create(parent=self.course, category='chapter', highlights=[u'good stuff']) + + def create_resolver(self): + """ + Creates a CourseUpdateResolver with an enrollment to schedule. + """ + with patch('openedx.core.djangoapps.schedules.signals.get_current_site') as mock_get_current_site: + mock_get_current_site.return_value = self.site_config.site + enrollment = CourseEnrollmentFactory(course_id=self.course.id, user=self.user, mode=u'audit') + + return CourseUpdateResolver( + async_send_task=Mock(name='async_send_task'), + site=self.site_config.site, + target_datetime=enrollment.schedule.start, + day_offset=-7, + bin_num=CourseUpdateResolver.bin_num_for_user_id(self.user.id), + ) + + @override_settings(CONTACT_MAILING_ADDRESS='123 Sesame Street') + @override_waffle_flag(COURSE_UPDATE_WAFFLE_FLAG, True) + def test_schedule_context(self): + resolver = self.create_resolver() + schedules = list(resolver.schedules_for_bin()) + expected_context = { + 'contact_email': 'info@example.com', + 'contact_mailing_address': '123 Sesame Street', + 'course_ids': [str(self.course.id)], + 'course_name': self.course.display_name, + 'course_url': '/courses/{}/course/'.format(self.course.id), + 'dashboard_url': '/dashboard', + 'homepage_url': '/', + 'mobile_store_urls': {}, + 'platform_name': u'\xe9dX', + 'show_upsell': False, + 'social_media_urls': {}, + 'template_revision': 'release', + 'unsubscribe_url': None, + 'week_highlights': ['good stuff'], + 'week_num': 1, + } + self.assertEqual(schedules, [(self.user, None, expected_context)]) + + @override_waffle_flag(COURSE_UPDATE_WAFFLE_FLAG, True) + @override_switch('schedules.course_update_show_unsubscribe', True) + def test_schedule_context_show_unsubscribe(self): + resolver = self.create_resolver() + schedules = list(resolver.schedules_for_bin()) + self.assertIn('optout', schedules[0][2]['unsubscribe_url']) diff --git a/openedx/core/djangolib/testing/utils.py b/openedx/core/djangolib/testing/utils.py index 81d38ad7a9..e194768225 100644 --- a/openedx/core/djangolib/testing/utils.py +++ b/openedx/core/djangolib/testing/utils.py @@ -48,6 +48,22 @@ class CacheIsolationMixin(object): __settings_overrides = [] __old_settings = [] + @classmethod + def setUpClass(cls): + super(CacheIsolationMixin, cls).setUpClass() + cls.start_cache_isolation() + + @classmethod + def tearDownClass(cls): + cls.end_cache_isolation() + super(CacheIsolationMixin, cls).tearDownClass() + + def setUp(self): + super(CacheIsolationMixin, self).setUp() + + self.clear_caches() + self.addCleanup(self.clear_caches) + @classmethod def start_cache_isolation(cls): """ @@ -131,21 +147,6 @@ class CacheIsolationTestCase(CacheIsolationMixin, TestCase): :py:class:`CacheIsolationMixin`) at class setup, and flushes the cache between every test. """ - @classmethod - def setUpClass(cls): - super(CacheIsolationTestCase, cls).setUpClass() - cls.start_cache_isolation() - - @classmethod - def tearDownClass(cls): - cls.end_cache_isolation() - super(CacheIsolationTestCase, cls).tearDownClass() - - def setUp(self): - super(CacheIsolationTestCase, self).setUp() - - self.clear_caches() - self.addCleanup(self.clear_caches) class _AssertNumQueriesContext(CaptureQueriesContext): From 2b80fdbf665eb4fc18807ef7fd1a31ceba733f27 Mon Sep 17 00:00:00 2001 From: Adeel Khan Date: Tue, 19 Nov 2019 08:42:15 +0500 Subject: [PATCH 06/20] Automate retry_failed_photo_verification mgt command This patch would enable a user to run management command via jenkins job. Verification ids are injected via a configuration model. PROD-1005 --- lms/djangoapps/verify_student/admin.py | 12 ++++- .../retry_failed_photo_verifications.py | 51 ++++++++++++++++--- .../commands/tests/test_verify_student.py | 45 +++++++++++++++- .../0012_sspverificationretryconfig.py | 31 +++++++++++ lms/djangoapps/verify_student/models.py | 24 ++++++++- 5 files changed, 153 insertions(+), 10 deletions(-) create mode 100644 lms/djangoapps/verify_student/migrations/0012_sspverificationretryconfig.py diff --git a/lms/djangoapps/verify_student/admin.py b/lms/djangoapps/verify_student/admin.py index 81b0ec64cc..971d2d689c 100644 --- a/lms/djangoapps/verify_student/admin.py +++ b/lms/djangoapps/verify_student/admin.py @@ -7,7 +7,9 @@ from __future__ import absolute_import from django.contrib import admin -from lms.djangoapps.verify_student.models import ManualVerification, SoftwareSecurePhotoVerification, SSOVerification +from lms.djangoapps.verify_student.models import ( + ManualVerification, SoftwareSecurePhotoVerification, SSOVerification, + SSPVerificationRetryConfig) @admin.register(SoftwareSecurePhotoVerification) @@ -39,3 +41,11 @@ class ManualVerificationAdmin(admin.ModelAdmin): list_display = ('id', 'user', 'status', 'reason', 'created_at', 'updated_at',) raw_id_fields = ('user',) search_fields = ('user__username', 'reason',) + + +@admin.register(SSPVerificationRetryConfig) +class SSPVerificationRetryAdmin(admin.ModelAdmin): + """ + Admin for the SSPVerificationRetryConfig table. + """ + pass diff --git a/lms/djangoapps/verify_student/management/commands/retry_failed_photo_verifications.py b/lms/djangoapps/verify_student/management/commands/retry_failed_photo_verifications.py index fd4c3cfbe0..3979e5d850 100644 --- a/lms/djangoapps/verify_student/management/commands/retry_failed_photo_verifications.py +++ b/lms/djangoapps/verify_student/management/commands/retry_failed_photo_verifications.py @@ -3,9 +3,13 @@ Django admin commands related to verify_student """ from __future__ import absolute_import, print_function +import logging from django.core.management.base import BaseCommand +from django.core.management.base import CommandError -from lms.djangoapps.verify_student.models import SoftwareSecurePhotoVerification +from lms.djangoapps.verify_student.models import SoftwareSecurePhotoVerification, SSPVerificationRetryConfig + +log = logging.getLogger('retry_photo_verification') class Command(BaseCommand): @@ -20,24 +24,59 @@ class Command(BaseCommand): "are in a state of 'must_retry'" ) + def add_arguments(self, parser): + + parser.add_argument( + '--verification-ids', + dest='verification_ids', + action='store', + nargs='+', + type=int, + help='verifications id used to retry verification' + ) + + parser.add_argument( + '--args-from-database', + action='store_true', + help='Use arguments from the SSPVerificationRetryConfig model instead of the command line.', + ) + + def get_args_from_database(self): + """ Returns an options dictionary from the current SSPVerificationRetryConfig model. """ + + sspv_retry_config = SSPVerificationRetryConfig.current() + if not sspv_retry_config.enabled: + raise CommandError('SSPVerificationRetryConfig is disabled, but --args-from-database was requested.') + + # We don't need fancy shell-style whitespace/quote handling - none of our arguments are complicated + argv = sspv_retry_config.arguments.split() + + parser = self.create_parser('manage.py', 'sspv_retry') + return parser.parse_args(argv).__dict__ # we want a dictionary, not a non-iterable Namespace object + def handle(self, *args, **options): + + options = self.get_args_from_database() if options['args_from_database'] else options + args = options.get('verification_ids', None) + if args: attempts_to_retry = SoftwareSecurePhotoVerification.objects.filter( - receipt_id__in=args + receipt_id__in=options['verification_ids'] ) + log.info(u"Fetching retry verification ids from config model") force_must_retry = True else: attempts_to_retry = SoftwareSecurePhotoVerification.objects.filter(status='must_retry') force_must_retry = False - print(u"Attempting to retry {0} failed PhotoVerification submissions".format(len(attempts_to_retry))) + log.info(u"Attempting to retry {0} failed PhotoVerification submissions".format(len(attempts_to_retry))) for index, attempt in enumerate(attempts_to_retry): - print(u"Retrying submission #{0} (ID: {1}, User: {2})".format(index, attempt.id, attempt.user)) + log.info(u"Retrying submission #{0} (ID: {1}, User: {2})".format(index, attempt.id, attempt.user)) # Set the attempts status to 'must_retry' so that we can re-submit it if force_must_retry: attempt.status = 'must_retry' attempt.submit(copy_id_photo_from=attempt.copy_id_photo_from) - print(u"Retry result: {0}".format(attempt.status)) - print("Done resubmitting failed photo verifications") + log.info(u"Retry result: {0}".format(attempt.status)) + log.info("Done resubmitting failed photo verifications") diff --git a/lms/djangoapps/verify_student/management/commands/tests/test_verify_student.py b/lms/djangoapps/verify_student/management/commands/tests/test_verify_student.py index 5f8fbd9979..986aa2ffe2 100644 --- a/lms/djangoapps/verify_student/management/commands/tests/test_verify_student.py +++ b/lms/djangoapps/verify_student/management/commands/tests/test_verify_student.py @@ -8,17 +8,21 @@ from __future__ import absolute_import import boto from django.conf import settings from django.core.management import call_command +from django.core.management.base import CommandError from django.test import TestCase from mock import patch +from testfixtures import LogCapture from common.test.utils import MockS3Mixin -from lms.djangoapps.verify_student.models import SoftwareSecurePhotoVerification +from lms.djangoapps.verify_student.models import SoftwareSecurePhotoVerification, SSPVerificationRetryConfig from lms.djangoapps.verify_student.tests.test_models import ( FAKE_SETTINGS, mock_software_secure_post, mock_software_secure_post_error ) -from student.tests.factories import UserFactory +from student.tests.factories import UserFactory # pylint: disable=import-error, useless-suppression + +LOGGER_NAME = 'retry_photo_verification' # Lots of patching to stub in our own settings, and HTTP posting @@ -64,3 +68,40 @@ class TestVerifyStudentCommand(MockS3Mixin, TestCase): call_command('retry_failed_photo_verifications') attempts_to_retry = SoftwareSecurePhotoVerification.objects.filter(status='must_retry') assert not attempts_to_retry + + def test_args_from_database(self): + """Test management command arguments injected from config model.""" + # Nothing in the database, should default to disabled + + # pylint: disable=deprecated-method, useless-suppression + with self.assertRaisesRegex(CommandError, 'SSPVerificationRetryConfig is disabled*'): + call_command('retry_failed_photo_verifications', '--args-from-database') + + # Add a config + config = SSPVerificationRetryConfig.current() + config.arguments = '--verification-ids 1 2 3' + config.enabled = True + config.save() + + with patch('lms.djangoapps.verify_student.models.requests.post', new=mock_software_secure_post_error): + self.create_and_submit("RetryRoger") + + with LogCapture(LOGGER_NAME) as log: + call_command('retry_failed_photo_verifications') + + log.check_present( + ( + LOGGER_NAME, 'INFO', + u"Attempting to retry {0} failed PhotoVerification submissions".format(1) + ), + ) + + with LogCapture(LOGGER_NAME) as log: + call_command('retry_failed_photo_verifications', '--args-from-database') + + log.check_present( + ( + LOGGER_NAME, 'INFO', + u"Fetching retry verification ids from config model" + ), + ) diff --git a/lms/djangoapps/verify_student/migrations/0012_sspverificationretryconfig.py b/lms/djangoapps/verify_student/migrations/0012_sspverificationretryconfig.py new file mode 100644 index 0000000000..2f150fdd4e --- /dev/null +++ b/lms/djangoapps/verify_student/migrations/0012_sspverificationretryconfig.py @@ -0,0 +1,31 @@ +# -*- coding: utf-8 -*- +# Generated by Django 1.11.26 on 2019-12-10 11:19 +from __future__ import unicode_literals + +from django.conf import settings +from django.db import migrations, models +import django.db.models.deletion + + +class Migration(migrations.Migration): + + dependencies = [ + migrations.swappable_dependency(settings.AUTH_USER_MODEL), + ('verify_student', '0011_add_fields_to_sspv'), + ] + + operations = [ + migrations.CreateModel( + name='SSPVerificationRetryConfig', + fields=[ + ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), + ('change_date', models.DateTimeField(auto_now_add=True, verbose_name='Change date')), + ('enabled', models.BooleanField(default=False, verbose_name='Enabled')), + ('arguments', models.TextField(blank=True, default='', help_text='Useful for manually running a Jenkins job. Specify like --verification-ids 1 2 3')), + ('changed_by', models.ForeignKey(editable=False, null=True, on_delete=django.db.models.deletion.PROTECT, to=settings.AUTH_USER_MODEL, verbose_name='Changed by')), + ], + options={ + 'verbose_name': 'sspv retry student argument', + }, + ), + ] diff --git a/lms/djangoapps/verify_student/models.py b/lms/djangoapps/verify_student/models.py index 3f6e3b8ca0..73f2e6b201 100644 --- a/lms/djangoapps/verify_student/models.py +++ b/lms/djangoapps/verify_student/models.py @@ -16,13 +16,14 @@ import functools import json import logging import os.path -import simplejson import uuid from datetime import timedelta from email.utils import formatdate import requests +import simplejson import six +from config_models.models import ConfigurationModel from django.conf import settings from django.contrib.auth.models import User from django.core.files.base import ContentFile @@ -44,6 +45,7 @@ from lms.djangoapps.verify_student.ssencrypt import ( ) from openedx.core.djangoapps.signals.signals import LEARNER_NOW_VERIFIED from openedx.core.storage import get_storage + from .utils import earliest_allowed_verification_date log = logging.getLogger(__name__) @@ -1108,3 +1110,23 @@ class VerificationDeadline(TimeStampedModel): return deadline.deadline except cls.DoesNotExist: return None + + +class SSPVerificationRetryConfig(ConfigurationModel): # pylint: disable=model-missing-unicode, useless-suppression + """ + SSPVerificationRetryConfig used to inject arguments + to retry_failed_photo_verifications management command + """ + + class Meta(object): + app_label = 'verify_student' + verbose_name = 'sspv retry student argument' + + arguments = models.TextField( + blank=True, + help_text='Useful for manually running a Jenkins job. Specify like --verification-ids 1 2 3', + default='' + ) + + def __str__(self): + return six.text_type(self.arguments) From c926a13f4124603539f5dc666a601e1e89036043 Mon Sep 17 00:00:00 2001 From: "hasnain.naveed" Date: Fri, 6 Dec 2019 12:59:49 +0500 Subject: [PATCH 07/20] ENT-1961 | Making the manual enrollment reason field optional via configuration flag `ENABLE_MANUAL_ENROLLMENT_REASON_FIELD`. --- .../tests/views/test_instructor_dashboard.py | 25 +++++++++++++++++++ .../instructor/views/instructor_dashboard.py | 3 ++- .../js/instructor_dashboard/membership.js | 2 +- .../instructor_dashboard_2/membership.html | 14 ++++++----- 4 files changed, 36 insertions(+), 8 deletions(-) diff --git a/lms/djangoapps/instructor/tests/views/test_instructor_dashboard.py b/lms/djangoapps/instructor/tests/views/test_instructor_dashboard.py index 6a14f60290..e3c159cfe7 100644 --- a/lms/djangoapps/instructor/tests/views/test_instructor_dashboard.py +++ b/lms/djangoapps/instructor/tests/views/test_instructor_dashboard.py @@ -158,6 +158,31 @@ class TestInstructorDashboard(ModuleStoreTestCase, LoginEnrollmentTestCase, XssT content('#field-course-organization b').contents()[0].strip() ) + @ddt.data(True, False) + def test_membership_reason_field_visibility(self, enbale_reason_field): + """ + Verify that reason field is enabled by site configuration flag 'ENABLE_MANUAL_ENROLLMENT_REASON_FIELD' + """ + + configuration_values = { + "ENABLE_MANUAL_ENROLLMENT_REASON_FIELD": enbale_reason_field + } + site = Site.objects.first() + SiteConfiguration.objects.create(site=site, values=configuration_values, enabled=True) + + url = reverse( + 'instructor_dashboard', + kwargs={ + 'course_id': six.text_type(self.course_info.id) + } + ) + response = self.client.get(url) + reason_field = '' # pylint: disable=line-too-long + if enbale_reason_field: + self.assertContains(response, reason_field) + else: + self.assertNotContains(response, reason_field) + def test_membership_site_configuration_role(self): """ Verify that the role choices set via site configuration are loaded in the membership tab diff --git a/lms/djangoapps/instructor/views/instructor_dashboard.py b/lms/djangoapps/instructor/views/instructor_dashboard.py index 37b3df7d9c..cbe1a38d3e 100644 --- a/lms/djangoapps/instructor/views/instructor_dashboard.py +++ b/lms/djangoapps/instructor/views/instructor_dashboard.py @@ -550,7 +550,8 @@ def _section_membership(course, access): 'update_forum_role_membership', kwargs={'course_id': six.text_type(course_key)} ), - 'enrollment_role_choices': enrollment_role_choices + 'enrollment_role_choices': enrollment_role_choices, + 'is_reason_field_enabled': configuration_helpers.get_value('ENABLE_MANUAL_ENROLLMENT_REASON_FIELD', False) } return section_data diff --git a/lms/static/js/instructor_dashboard/membership.js b/lms/static/js/instructor_dashboard/membership.js index 1f3164dd4b..a9e5bf73a2 100644 --- a/lms/static/js/instructor_dashboard/membership.js +++ b/lms/static/js/instructor_dashboard/membership.js @@ -603,7 +603,7 @@ such that the value can be defined later than this assignment (file load order). this.$request_response_error = this.$container.find('.request-response-error'); this.$enrollment_button.click(function(event) { var sendData; - if (!batchEnroll.$reason_field.val()) { + if (batchEnroll.$reason_field.length && !batchEnroll.$reason_field.val()) { batchEnroll.fail_with_error(gettext('Reason field should not be left blank.')); return false; } diff --git a/lms/templates/instructor/instructor_dashboard_2/membership.html b/lms/templates/instructor/instructor_dashboard_2/membership.html index 2662ad958e..84ac8b7b1f 100644 --- a/lms/templates/instructor/instructor_dashboard_2/membership.html +++ b/lms/templates/instructor/instructor_dashboard_2/membership.html @@ -24,12 +24,14 @@ from openedx.core.djangolib.markup import HTML, Text - + % if section_data['is_reason_field_enabled']: + + %endif
% else:
-

${_("Your final grade:")} - ${"{0:.0f}%".format(float(cert_status['grade'])*100)}. - +

+ % if should_display_grade(course_overview.end): + ${_("Your final grade:")} + ${"{0:.0f}%".format(float(cert_status['grade'])*100)}. + % endif % if cert_status['status'] == 'notpassing': % if enrollment.mode != 'audit': ${_("Grade required for a {cert_name_short}:").format(cert_name_short=cert_name_short)} From 7eb21f4dec525bdf2dacfb1807c2d089eb989884 Mon Sep 17 00:00:00 2001 From: Zainab Amir <40633976+zainab-amir@users.noreply.github.com> Date: Thu, 12 Dec 2019 18:07:34 +0500 Subject: [PATCH 16/20] Change version of social-auth-app-django (#21956) Microsoft social login is not working on edx mobile app. The issue is fixed in newer version of social-auth-app-django. PROD-718 --- .../djangoapps/third_party_auth/pipeline.py | 9 ++++ .../third_party_auth/tests/specs/base.py | 45 +++++++++++-------- .../tests/specs/test_linkedin.py | 33 ++++++++++++-- .../third_party_auth/tests/utils.py | 2 +- requirements/edx/base.in | 4 +- requirements/edx/base.txt | 4 +- requirements/edx/development.txt | 4 +- requirements/edx/testing.txt | 4 +- 8 files changed, 75 insertions(+), 30 deletions(-) diff --git a/common/djangoapps/third_party_auth/pipeline.py b/common/djangoapps/third_party_auth/pipeline.py index b4a683e9b5..c3a17d642f 100644 --- a/common/djangoapps/third_party_auth/pipeline.py +++ b/common/djangoapps/third_party_auth/pipeline.py @@ -209,6 +209,11 @@ def get(request): """Gets the running pipeline's data from the passed request.""" strategy = social_django.utils.load_strategy(request) token = strategy.session_get('partial_pipeline_token') + + if not token: + strategy.session_set('partial_pipeline_token', strategy.session_get('partial_pipeline_token_')) + token = strategy.session_get('partial_pipeline_token') + partial_object = strategy.partial_load(token) pipeline_data = None if partial_object: @@ -560,6 +565,10 @@ def ensure_user_information(strategy, auth_entry, backend=None, user=None, socia return (current_provider and current_provider.slug in [saml_provider.slug for saml_provider in saml_providers_list]) + if current_partial: + strategy.session_set('partial_pipeline_token_', current_partial.token) + strategy.storage.partial.store(current_partial) + if not user: # Use only email for user existence check in case of saml provider if is_provider_saml(): diff --git a/common/djangoapps/third_party_auth/tests/specs/base.py b/common/djangoapps/third_party_auth/tests/specs/base.py index f29099308d..73521de0f5 100644 --- a/common/djangoapps/third_party_auth/tests/specs/base.py +++ b/common/djangoapps/third_party_auth/tests/specs/base.py @@ -542,6 +542,8 @@ class IntegrationTest(testutil.TestCase, test.TestCase, HelperMixin): request.backend.auth_complete = mock.MagicMock(return_value=self.fake_auth_complete(strategy)) request.user = self.create_user_models_for_existing_account( strategy, 'user@example.com', 'password', self.get_username(), skip_social_auth=True) + partial_pipeline_token = strategy.session_get('partial_pipeline_token') + partial_data = strategy.storage.partial.load(partial_pipeline_token) # Instrument the pipeline to get to the dashboard with the full # expected state. @@ -561,24 +563,14 @@ class IntegrationTest(testutil.TestCase, test.TestCase, HelperMixin): # We should be redirected back to the complete page, setting # the "logged in" cookie for the marketing site. - self.assert_logged_in_cookie_redirect(actions.do_complete( - request.backend, social_views._do_login, request.user, None, # pylint: disable=protected-access - redirect_field_name=auth.REDIRECT_FIELD_NAME, request=request - )) + self.assert_logged_in_cookie_redirect(self.do_complete(strategy, request, partial_pipeline_token, partial_data)) # Set the cookie and try again self.set_logged_in_cookies(request) # Fire off the auth pipeline to link. self.assert_redirect_after_pipeline_completes( - actions.do_complete( - request.backend, - social_views._do_login, # pylint: disable=protected-access - request.user, - None, - redirect_field_name=auth.REDIRECT_FIELD_NAME, - request=request - ) + self.do_complete(strategy, request, partial_pipeline_token, partial_data) ) # Now we expect to be in the linked state, with a backend entry. @@ -694,6 +686,9 @@ class IntegrationTest(testutil.TestCase, test.TestCase, HelperMixin): strategy.request.backend.auth_complete = mock.MagicMock(return_value=self.fake_auth_complete(strategy)) user = self.create_user_models_for_existing_account( strategy, 'user@example.com', 'password', self.get_username()) + partial_pipeline_token = strategy.session_get('partial_pipeline_token') + partial_data = strategy.storage.partial.load(partial_pipeline_token) + self.assert_social_auth_exists_for_user(user, strategy) self.assertTrue(user.is_active) @@ -734,7 +729,8 @@ class IntegrationTest(testutil.TestCase, test.TestCase, HelperMixin): self.set_logged_in_cookies(request) self.assert_redirect_after_pipeline_completes( - actions.do_complete(request.backend, social_views._do_login, user=user, request=request)) + self.do_complete(strategy, request, partial_pipeline_token, partial_data, user) + ) self.assert_account_settings_context_looks_correct(account_settings_context(request)) def test_signin_fails_if_account_not_active(self): @@ -793,6 +789,8 @@ class IntegrationTest(testutil.TestCase, test.TestCase, HelperMixin): request, strategy = self.get_request_and_strategy( auth_entry=pipeline.AUTH_ENTRY_REGISTER, redirect_uri='social:complete') strategy.request.backend.auth_complete = mock.MagicMock(return_value=self.fake_auth_complete(strategy)) + partial_pipeline_token = strategy.session_get('partial_pipeline_token') + partial_data = strategy.storage.partial.load(partial_pipeline_token) # Begin! Grab the registration page and check the login control on it. self.assert_register_response_before_pipeline_looks_correct(self.client.get('/register')) @@ -846,15 +844,13 @@ class IntegrationTest(testutil.TestCase, test.TestCase, HelperMixin): # We should be redirected back to the complete page, setting # the "logged in" cookie for the marketing site. - self.assert_logged_in_cookie_redirect(actions.do_complete( - request.backend, social_views._do_login, request.user, None, # pylint: disable=protected-access - redirect_field_name=auth.REDIRECT_FIELD_NAME, request=request - )) + self.assert_logged_in_cookie_redirect(self.do_complete(strategy, request, partial_pipeline_token, partial_data)) # Set the cookie and try again self.set_logged_in_cookies(request) self.assert_redirect_after_pipeline_completes( - actions.do_complete(strategy.request.backend, social_views._do_login, user=created_user, request=request)) + self.do_complete(strategy, request, partial_pipeline_token, partial_data, created_user) + ) # Now the user has been redirected to the dashboard. Their third party account should now be linked. self.assert_social_auth_exists_for_user(created_user, strategy) self.assert_account_settings_context_looks_correct(account_settings_context(request), linked=True) @@ -974,6 +970,19 @@ class IntegrationTest(testutil.TestCase, test.TestCase, HelperMixin): """ raise NotImplementedError + def do_complete(self, strategy, request, partial_pipeline_token, partial_data, user=None): + """ + Makes sure that strategy store includes the partial data object before + calling actions.do_complete + """ + strategy.storage.partial.store(partial_data) + if not user: + user = request.user + return actions.do_complete( + request.backend, social_views._do_login, user, None, # pylint: disable=protected-access + redirect_field_name=auth.REDIRECT_FIELD_NAME, request=request, partial_token=partial_pipeline_token + ) + # pylint: disable=abstract-method @django_utils.override_settings(ECOMMERCE_API_URL=TEST_API_URL) diff --git a/common/djangoapps/third_party_auth/tests/specs/test_linkedin.py b/common/djangoapps/third_party_auth/tests/specs/test_linkedin.py index ac4f47d895..091471657d 100644 --- a/common/djangoapps/third_party_auth/tests/specs/test_linkedin.py +++ b/common/djangoapps/third_party_auth/tests/specs/test_linkedin.py @@ -4,6 +4,15 @@ from __future__ import absolute_import from third_party_auth.tests.specs import base +def get_localized_name(name): + """Returns the localizedName from the name object""" + locale = "{}_{}".format( + name["preferredLocale"]["language"], + name["preferredLocale"]["country"] + ) + return name['localized'].get(locale, '') + + class LinkedInOauth2IntegrationTest(base.Oauth2IntegrationTest): """Integration tests for provider.LinkedInOauth2.""" @@ -21,11 +30,29 @@ class LinkedInOauth2IntegrationTest(base.Oauth2IntegrationTest): 'expires_in': 'expires_in_value', } USER_RESPONSE_DATA = { - 'lastName': 'lastName_value', + 'lastName': { + "localized": { + "en_US": "Doe" + }, + "preferredLocale": { + "country": "US", + "language": "en" + } + }, 'id': 'id_value', - 'firstName': 'firstName_value', + 'firstName': { + "localized": { + "en_US": "Doe" + }, + "preferredLocale": { + "country": "US", + "language": "en" + } + }, } def get_username(self): response_data = self.get_response_data() - return response_data.get('firstName') + response_data.get('lastName') + first_name = get_localized_name(response_data.get('firstName')) + last_name = get_localized_name(response_data.get('lastName')) + return first_name + last_name diff --git a/common/djangoapps/third_party_auth/tests/utils.py b/common/djangoapps/third_party_auth/tests/utils.py index 8fe9ee136a..47b8bb5173 100644 --- a/common/djangoapps/third_party_auth/tests/utils.py +++ b/common/djangoapps/third_party_auth/tests/utils.py @@ -98,7 +98,7 @@ class ThirdPartyOAuthTestMixinFacebook(object): class ThirdPartyOAuthTestMixinGoogle(object): """Tests oauth with the Google backend""" BACKEND = "google-oauth2" - USER_URL = "https://www.googleapis.com/plus/v1/people/me" + USER_URL = "https://www.googleapis.com/oauth2/v3/userinfo" # In google-oauth2 responses, the "email" field is used as the user's identifier UID_FIELD = "email" diff --git a/requirements/edx/base.in b/requirements/edx/base.in index fb89cec707..781ae4a5e4 100644 --- a/requirements/edx/base.in +++ b/requirements/edx/base.in @@ -133,8 +133,8 @@ pyuca==1.1 # For more accurate sorting of translated co recommender-xblock # https://github.com/edx/RecommenderXBlock rest-condition # DRF's recommendation for supporting complex permissions rfc6266-parser # Used to generate Content-Disposition headers. -social-auth-app-django<3.0.0 -social-auth-core<2.0.0 +social-auth-app-django==3.1.0 +social-auth-core==3.2.0 pysrt # Support for SubRip subtitle files, used in the video XModule pytz # Time zone information database PyYAML # Used to parse XModule resource templates diff --git a/requirements/edx/base.txt b/requirements/edx/base.txt index a2419357d7..4ab31d76b2 100644 --- a/requirements/edx/base.txt +++ b/requirements/edx/base.txt @@ -227,8 +227,8 @@ simplejson==3.17.0 singledispatch==3.4.0.3 six==1.13.0 slumber==0.7.1 # via edx-bulk-grades, edx-enterprise, edx-rest-api-client -social-auth-app-django==2.1.0 -social-auth-core==1.7.0 +social-auth-app-django==3.1.0 +social-auth-core==3.2.0 sorl-thumbnail==12.3 sortedcontainers==2.1.0 soupsieve==1.9.5 # via beautifulsoup4 diff --git a/requirements/edx/development.txt b/requirements/edx/development.txt index 151a45b367..9007b65d3f 100644 --- a/requirements/edx/development.txt +++ b/requirements/edx/development.txt @@ -305,8 +305,8 @@ singledispatch==3.4.0.3 six==1.13.0 slumber==0.7.1 snowballstemmer==2.0.0 # via sphinx -social-auth-app-django==2.1.0 -social-auth-core==1.7.0 +social-auth-app-django==3.1.0 +social-auth-core==3.2.0 sorl-thumbnail==12.3 sortedcontainers==2.1.0 soupsieve==1.9.5 diff --git a/requirements/edx/testing.txt b/requirements/edx/testing.txt index d9dea4233a..70e3d991eb 100644 --- a/requirements/edx/testing.txt +++ b/requirements/edx/testing.txt @@ -292,8 +292,8 @@ simplejson==3.17.0 singledispatch==3.4.0.3 six==1.13.0 slumber==0.7.1 -social-auth-app-django==2.1.0 -social-auth-core==1.7.0 +social-auth-app-django==3.1.0 +social-auth-core==3.2.0 sorl-thumbnail==12.3 sortedcontainers==2.1.0 soupsieve==1.9.5 From d79e7df32bad8fcb54dd70fc2c3183ae38cf1d86 Mon Sep 17 00:00:00 2001 From: Robert Raposa Date: Fri, 6 Dec 2019 15:34:50 -0500 Subject: [PATCH 17/20] use login_ajax for logistration - use login_ajax (in place of login_session with shim) for logistration's call to login POST - add toggle for using login_ajax from logistration - FEATURES['ENABLE_LOGIN_POST_WITHOUT_SHIM'] - add custom metrics for redirect_url - update test for third-party auth error_code NOTE: The error_code `third-party-auth-with-no-linked-account` was introduced in JSON in this earlier PR: https://github.com/edx/edx-platform/pull/22452/files ARCH-1253 --- .../third_party_auth/tests/specs/base.py | 9 ++-- .../js/student_account/views/LoginView.js | 49 ++++++++++++++++++- .../core/djangoapps/user_authn/views/login.py | 8 +++ .../djangoapps/user_authn/views/login_form.py | 21 +++++++- .../user_authn/views/tests/test_login.py | 22 +++++++-- 5 files changed, 96 insertions(+), 13 deletions(-) diff --git a/common/djangoapps/third_party_auth/tests/specs/base.py b/common/djangoapps/third_party_auth/tests/specs/base.py index 73521de0f5..c2af79b603 100644 --- a/common/djangoapps/third_party_auth/tests/specs/base.py +++ b/common/djangoapps/third_party_auth/tests/specs/base.py @@ -130,11 +130,10 @@ class HelperMixin(object): def assert_json_failure_response_is_missing_social_auth(self, response): """Asserts failure on /login for missing social auth looks right.""" - self.assertContains( - response, - u"successfully signed in to your %s account, but this account isn't linked" % self.provider.name, - status_code=403, - ) + self.assertEqual(403, response.status_code) + payload = json.loads(response.content.decode('utf-8')) + self.assertFalse(payload.get('success')) + self.assertEqual(payload.get('error_code'), 'third-party-auth-with-no-linked-account') def assert_json_failure_response_is_username_collision(self, response): """Asserts the json response indicates a username collision.""" diff --git a/lms/static/js/student_account/views/LoginView.js b/lms/static/js/student_account/views/LoginView.js index 477aa7d197..7fe1f004ec 100644 --- a/lms/static/js/student_account/views/LoginView.js +++ b/lms/static/js/student_account/views/LoginView.js @@ -189,6 +189,51 @@ }, saveError: function(error) { + if (error.responseJSON !== undefined) { + this.saveErrorWithoutShim(error); + } else { + this.saveErrorWithShim(error); + } + }, + + saveErrorWithoutShim: function(error) { + var errorCode; + var msg; + if (error.status === 0) { + msg = gettext('An error has occurred. Check your Internet connection and try again.'); + } else if (error.status === 500) { + msg = gettext('An error has occurred. Try refreshing the page, or check your Internet connection.'); // eslint-disable-line max-len + } else if (error.responseJSON !== undefined) { + msg = error.responseJSON.value; + errorCode = error.responseJSON.error_code; + } else { + msg = gettext('An unexpected error has occurred.'); + } + + this.errors = [ + StringUtils.interpolate( + '

  • {msg}
  • ', { + msg: msg + } + ) + ]; + this.clearPasswordResetSuccess(); + + /* If the user successfully authenticated with a third-party provider, but they haven't + * linked the accounts, instruct the user on how to link the accounts. + */ + if (errorCode === 'third-party-auth-with-no-linked-account' && this.currentProvider) { + if (!this.hideAuthWarnings) { + this.clearFormErrors(); + this.renderThirdPartyAuthWarning(); + } + } else { + this.renderErrors(this.defaultFormErrorsTitle, this.errors); + } + this.toggleDisableButton(false); + }, + + saveErrorWithShim: function(error) { var msg = error.responseText; if (error.status === 0) { msg = gettext('An error has occurred. Check your Internet connection and try again.'); @@ -215,7 +260,7 @@ this.currentProvider) { if (!this.hideAuthWarnings) { this.clearFormErrors(); - this.renderAuthWarning(); + this.renderThirdPartyAuthWarning(); } } else { this.renderErrors(this.defaultFormErrorsTitle, this.errors); @@ -223,7 +268,7 @@ this.toggleDisableButton(false); }, - renderAuthWarning: function() { + renderThirdPartyAuthWarning: function() { var message = _.sprintf( gettext('You have successfully signed into %(currentProvider)s, but your %(currentProvider)s' + ' account does not have a linked %(platformName)s account. To link your accounts,' + diff --git a/openedx/core/djangoapps/user_authn/views/login.py b/openedx/core/djangoapps/user_authn/views/login.py index 268b5f5365..32a96e6c5c 100644 --- a/openedx/core/djangoapps/user_authn/views/login.py +++ b/openedx/core/djangoapps/user_authn/views/login.py @@ -400,6 +400,7 @@ def login_user(request): response = set_logged_in_cookies(request, response, possibly_authenticated_user) set_custom_metric('login_user_auth_failed_error', False) set_custom_metric('login_user_response_status', response.status_code) + set_custom_metric('login_user_redirect_url', redirect_url) return response except AuthFailedError as error: log.exception(error.get_response()) @@ -483,10 +484,15 @@ def _parse_analytics_param_for_course_id(request): modified_request = request.POST.copy() if isinstance(request, HttpRequest): # Works for an HttpRequest but not a rest_framework.request.Request. + # Note: This case seems to be used for tests only. request.POST = modified_request + set_custom_metric('login_user_request_type', 'django') else: # The request must be a rest_framework.request.Request. + # Note: Only DRF seems to be used in Production. request._data = modified_request # pylint: disable=protected-access + set_custom_metric('login_user_request_type', 'drf') + # Include the course ID if it's specified in the analytics info # so it can be included in analytics events. if "analytics" in modified_request: @@ -566,6 +572,8 @@ def shim_student_view(view_func, check_logged_in=False): msg = response_dict.get("value", u"") success = response_dict.get("success") set_custom_metric('shim_original_response_is_json', True) + set_custom_metric('shim_original_redirect_url', response_dict.get("redirect_url")) + set_custom_metric('shim_original_redirect', response_dict.get("redirect")) except (ValueError, TypeError): msg = response.content success = True diff --git a/openedx/core/djangoapps/user_authn/views/login_form.py b/openedx/core/djangoapps/user_authn/views/login_form.py index 6397103850..faeab3ef95 100644 --- a/openedx/core/djangoapps/user_authn/views/login_form.py +++ b/openedx/core/djangoapps/user_authn/views/login_form.py @@ -77,6 +77,20 @@ def _apply_third_party_auth_overrides(request, form_desc): ) +# .. toggle_name: FEATURES[ENABLE_LOGIN_POST_WITHOUT_SHIM] +# .. toggle_implementation: DjangoSetting +# .. toggle_default: False +# .. toggle_description: Toggle for enabling login post without shim_student_view (using `login_api`). +# .. toggle_category: n/a +# .. toggle_use_cases: incremental_release +# .. toggle_creation_date: 2019-12-10 +# .. toggle_expiration_date: 2020-06-01 +# .. toggle_warnings: n/a +# .. toggle_tickets: ARCH-1253 +# .. toggle_status: supported +ENABLE_LOGIN_POST_WITHOUT_SHIM = 'ENABLE_LOGIN_POST_WITHOUT_SHIM' + + def get_login_session_form(request): """Return a description of the login form. @@ -91,7 +105,12 @@ def get_login_session_form(request): HttpResponse """ - form_desc = FormDescription("post", reverse("user_api_login_session")) + if settings.FEATURES.get(ENABLE_LOGIN_POST_WITHOUT_SHIM): + submit_url = reverse("login_api") + else: + submit_url = reverse("user_api_login_session") + + form_desc = FormDescription("post", submit_url) _apply_third_party_auth_overrides(request, form_desc) # Translators: This label appears above a field on the login form diff --git a/openedx/core/djangoapps/user_authn/views/tests/test_login.py b/openedx/core/djangoapps/user_authn/views/tests/test_login.py index 49e6d84cda..df33328efa 100644 --- a/openedx/core/djangoapps/user_authn/views/tests/test_login.py +++ b/openedx/core/djangoapps/user_authn/views/tests/test_login.py @@ -34,6 +34,7 @@ from openedx.core.djangoapps.user_authn.views.login import ( AllowedAuthUser, ENABLE_LOGIN_USING_THIRDPARTY_AUTH_ONLY ) +from openedx.core.djangoapps.user_authn.views.login_form import ENABLE_LOGIN_POST_WITHOUT_SHIM from openedx.core.djangoapps.user_authn.tests.utils import setup_login_oauth_client from openedx.core.djangolib.testing.utils import CacheIsolationTestCase, skip_unless_lms from openedx.core.djangoapps.site_configuration.tests.mixins import SiteMixin @@ -661,15 +662,26 @@ class LoginSessionViewTest(ApiTestCase): response = self.client.patch(self.url) self.assertHttpMethodNotAllowed(response) - def test_login_form(self): - # Retrieve the login form - response = self.client.get(self.url, content_type="application/json") - self.assertHttpOK(response) + @ddt.data( + {ENABLE_LOGIN_POST_WITHOUT_SHIM: True}, + {ENABLE_LOGIN_POST_WITHOUT_SHIM: False}, + {}, + ) + def test_login_form(self, features_setting): + with patch.dict("django.conf.settings.FEATURES", features_setting): + # Retrieve the login form + response = self.client.get(self.url, content_type="application/json") + self.assertHttpOK(response) + + if ENABLE_LOGIN_POST_WITHOUT_SHIM in features_setting and features_setting[ENABLE_LOGIN_POST_WITHOUT_SHIM]: + submit_url = reverse("login_api") + else: + submit_url = reverse("user_api_login_session") # Verify that the form description matches what we expect form_desc = json.loads(response.content.decode('utf-8')) self.assertEqual(form_desc["method"], "post") - self.assertEqual(form_desc["submit_url"], self.url) + self.assertEqual(form_desc["submit_url"], submit_url) self.assertEqual(form_desc["fields"], [ { "name": "email", From 842adc6fcd1a7d233d3302ed7042b11261c407c8 Mon Sep 17 00:00:00 2001 From: Calen Pennington Date: Thu, 12 Dec 2019 13:45:00 -0500 Subject: [PATCH 18/20] Archive WTW artifacts and send splunk files at the end of the WTW collection phase --- scripts/Jenkinsfiles/bokchoy | 37 +++++++++++++++++------------------- 1 file changed, 17 insertions(+), 20 deletions(-) diff --git a/scripts/Jenkinsfiles/bokchoy b/scripts/Jenkinsfiles/bokchoy index 3dafd9148e..f8bcffdb53 100644 --- a/scripts/Jenkinsfiles/bokchoy +++ b/scripts/Jenkinsfiles/bokchoy @@ -92,28 +92,25 @@ pipeline { steps { script { sshagent(credentials: ['jenkins-worker'], ignoreMissing: true) { - checkout changelog: false, poll: false, scm: [$class: 'GitSCM', branches: [[name: git_branch]], - doGenerateSubmoduleConfigurations: false, extensions: [[$class: 'CloneOption', honorRefspec: true, - noTags: true, shallow: true]], submoduleCfg: [], userRemoteConfigs: [[credentialsId: 'jenkins-worker', - refspec: git_refspec, url: "git@github.com:edx/${REPO_NAME}.git"]]] + try { + checkout changelog: false, poll: false, scm: [$class: 'GitSCM', branches: [[name: git_branch]], + doGenerateSubmoduleConfigurations: false, extensions: [[$class: 'CloneOption', honorRefspec: true, + noTags: true, shallow: true]], submoduleCfg: [], userRemoteConfigs: [[credentialsId: 'jenkins-worker', + refspec: git_refspec, url: "git@github.com:edx/${REPO_NAME}.git"]]] - for (int i = 1; i <= shardCount; i++) { - unstash "bok-choy-reports-${i}" + for (int i = 1; i <= shardCount; i++) { + unstash "bok-choy-reports-${i}" + } + sh """ + export TEST_SUITE=bok-choy + source scripts/jenkins-common.sh + paver coverage --rcfile=common/test/acceptance/.coveragerc + paver upload_coverage_to_s3 + """ + } finally { + archiveArtifacts allowEmptyArchive: true, artifacts: 'reports/*.coverage' + sendSplunkFile excludes: '', includes: '**/timing*.log', sizeLimit: '10MB' } - sh """ - export TEST_SUITE=bok-choy - source scripts/jenkins-common.sh - paver coverage --rcfile=common/test/acceptance/.coveragerc - paver upload_coverage_to_s3 - """ - } - } - } - post { - always { - script { - archiveArtifacts allowEmptyArchive: true, artifacts: 'reports/*.coverage' - sendSplunkFile excludes: '', includes: '**/timing*.log', sizeLimit: '10MB' } } } From 30c8e1571e7ce8482d8dc09d2b32b7c63621f3ee Mon Sep 17 00:00:00 2001 From: asadazam93 Date: Wed, 6 Nov 2019 18:00:13 +0500 Subject: [PATCH 19/20] Export staff users csv --- .../management/commands/export_staff_users.py | 134 ++++++++++++++++++ .../tests/test_export_staff_users.py | 35 +++++ .../student/templates/email/email_base.html | 21 +++ .../templates/email/export_staff_users.html | 15 ++ .../templates/email/export_staff_users.txt | 7 + 5 files changed, 212 insertions(+) create mode 100644 common/djangoapps/student/management/commands/export_staff_users.py create mode 100644 common/djangoapps/student/management/tests/test_export_staff_users.py create mode 100644 common/djangoapps/student/templates/email/email_base.html create mode 100644 common/djangoapps/student/templates/email/export_staff_users.html create mode 100644 common/djangoapps/student/templates/email/export_staff_users.txt diff --git a/common/djangoapps/student/management/commands/export_staff_users.py b/common/djangoapps/student/management/commands/export_staff_users.py new file mode 100644 index 0000000000..964f1dc712 --- /dev/null +++ b/common/djangoapps/student/management/commands/export_staff_users.py @@ -0,0 +1,134 @@ +from __future__ import absolute_import, print_function + +import csv +import logging +from datetime import datetime, timedelta +from django.core.management.base import BaseCommand +from django.conf import settings +from django.core.mail.message import EmailMultiAlternatives +from django.template.loader import get_template +from pytz import utc +from os import remove + +from openedx.core.djangoapps.content.course_overviews.models import CourseOverview +from student.models import CourseAccessRole + +logger = logging.getLogger(__name__) + + +class Command(BaseCommand): + """ + Example usage: + $ ./manage.py lms export_staff_users -d 7 --settings=devstack_docker + $ ./manage.py lms export_staff_users --days 7 --settings=devstack_docker + $ ./manage.py lms export_staff_users --days 7 --dry true --settings=devstack_docker + """ + + help = """ + This command will export a csv of all users who have logged in within the given days and + have staff access role in active courses (Courses with end date in the future). + """ + + def add_arguments(self, parser): + parser.add_argument( + '-d', + '--days', + type=int, + default=7, + help='Indicate the login time period in days starting from today' + ) + parser.add_argument( + '-r', + '--dry', + type=str, + help='Indicate that the email should not be sent to author-support' + ) + + subject = 'Staff users CSV' + to_addresses = ['author-support@edx.org'] + from_address = settings.DEFAULT_FROM_EMAIL + txt_template_path = 'email/export_staff_users.txt' + html_template_path = 'email/export_staff_users.html' + csv_filename = 'staff_users.csv' + + def write_csv(self, query_set, filename): + """ + Writes the queryset into a csv file with the given filename + + Arguments: + query_set: query_set to be converted + filename: filename for the csv + """ + writer = csv.DictWriter( + filename, + fieldnames=['id', 'user__username', 'user__email', 'role'] + ) + writer.writeheader() + for data_item in query_set: + writer.writerow(data_item) + + def handle(self, *args, **kwargs): + days = kwargs['days'] + dry = kwargs.get('dry') + if dry: + self.to_addresses = ['sustaining-mavericks@edx.org'] + current_date = datetime.now(tz=utc) + starting_date = current_date - timedelta(days=days) + active_courses = CourseOverview.objects.filter(end__gte=current_date).values_list('id', flat=True) + course_access_roles = CourseAccessRole.objects.filter( + role__in=['staff', 'instructor'], + user__last_login__range=(starting_date, current_date), + course_id__in=active_courses, + user__is_staff=False + ).values('id', 'user__username', 'user__email', 'role') + if not course_access_roles: + return + with open(self.csv_filename, 'a+') as csv_file: + self.write_csv( + query_set=course_access_roles, + filename=csv_file + ) + context = {'time_period': days} + try: + self.send_email(context) + logger.info( + 'Sent staff users email for the period {} to {}. Staff users count:{}'.format( + starting_date, + current_date, + course_access_roles.count() + ) + ) + except Exception: + logger.exception( + 'Failed to send staff users email for the period {}-{}'.format(starting_date, current_date) + ) + + def send_email(self, context): + """ + Sends an email to admin containing a csv of all users who have logged in within the given days and + have staff access role in active courses (Courses with end date in the future). + + Arguments: + context: context for the email template + """ + plain_content = self.render_template(self.txt_template_path, context) + html_content = self.render_template(self.html_template_path, context) + + with open(self.csv_filename, 'r') as csv_file: + email_message = EmailMultiAlternatives(self.subject, plain_content, self.from_address, to=self.to_addresses) + email_message.attach_alternative(html_content, 'text/html') + email_message.attach(self.csv_filename, csv_file.read(), 'text/csv') + email_message.send() + + remove(self.csv_filename) + + def render_template(self, path, context): + """ + Takes a template path and context and returns a rendered template + + Arguments: + path: path of the file + context: context for the template + """ + txt_template = get_template(path) + return txt_template.render(context) diff --git a/common/djangoapps/student/management/tests/test_export_staff_users.py b/common/djangoapps/student/management/tests/test_export_staff_users.py new file mode 100644 index 0000000000..9afc3f667c --- /dev/null +++ b/common/djangoapps/student/management/tests/test_export_staff_users.py @@ -0,0 +1,35 @@ +""" +Unit tests for export_staff_users management command. +""" +from datetime import timedelta + +from django.core import mail +from django.core.management import call_command +from django.test import TestCase +from django.utils.timezone import now + +from openedx.core.djangoapps.content.course_overviews.tests.factories import CourseOverviewFactory +from student.tests.factories import CourseAccessRoleFactory, UserFactory + + +class TestExportStaffUsers(TestCase): + """ + Tests the `export_staff_users` command. + """ + + @staticmethod + def create_users_data(): + staff_user = UserFactory(last_login=now() - timedelta(days=5)) + instructor_user = UserFactory(last_login=now() - timedelta(days=5)) + course = CourseOverviewFactory(end=now() + timedelta(days=30)) + archived_course = CourseOverviewFactory(end=now() - timedelta(days=30)) + course_ids = [course.id, archived_course.id] + for course_id in course_ids: + CourseAccessRoleFactory.create(course_id=course_id, user=staff_user, role="staff") + CourseAccessRoleFactory.create(course_id=course_id, user=instructor_user, role="instructor") + + def test_export_staff_users(self): + self.create_users_data() + self.assertEqual(len(mail.outbox), 0) + call_command('export_staff_users', days=7) + self.assertEqual(len(mail.outbox), 1) diff --git a/common/djangoapps/student/templates/email/email_base.html b/common/djangoapps/student/templates/email/email_base.html new file mode 100644 index 0000000000..0cf1739f84 --- /dev/null +++ b/common/djangoapps/student/templates/email/email_base.html @@ -0,0 +1,21 @@ +{% load i18n %} +{% get_current_language as LANGUAGE_CODE %} + + + + + + + + + + + + + +
    + {% block body %} + {% endblock body %} +
    + + diff --git a/common/djangoapps/student/templates/email/export_staff_users.html b/common/djangoapps/student/templates/email/export_staff_users.html new file mode 100644 index 0000000000..c54f5a52b7 --- /dev/null +++ b/common/djangoapps/student/templates/email/export_staff_users.html @@ -0,0 +1,15 @@ +{% extends "email/email_base.html" %} +{% block body %} + +

    + Dear Admin, +

    +

    + Please find the attached CSV containing a list of all staff users + who have logged in within the last {{ time_period }} days +

    + +

    Thanks,

    +

    The edX Team

    + +{% endblock body %} diff --git a/common/djangoapps/student/templates/email/export_staff_users.txt b/common/djangoapps/student/templates/email/export_staff_users.txt new file mode 100644 index 0000000000..cda5918ceb --- /dev/null +++ b/common/djangoapps/student/templates/email/export_staff_users.txt @@ -0,0 +1,7 @@ +Dear Admin, + + Please find the attached CSV containing a list of all staff users who have logged in within the last {{ time_period }} days + + +Thanks, +The edX Team From c545ed8bbe077e8879f3c8ae730e8a565006f501 Mon Sep 17 00:00:00 2001 From: uzairr Date: Fri, 13 Dec 2019 12:47:15 +0500 Subject: [PATCH 20/20] Update privacy and TOS links for edge. Currently,edge environment is serving empty pages against privacy and TOS.Now, this behaviour is modified by pointing those links to edX's privacy and TOS pages. PROD-1066 --- common/djangoapps/edxmako/shortcuts.py | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/common/djangoapps/edxmako/shortcuts.py b/common/djangoapps/edxmako/shortcuts.py index e5a2c639eb..64838852a4 100644 --- a/common/djangoapps/edxmako/shortcuts.py +++ b/common/djangoapps/edxmako/shortcuts.py @@ -15,16 +15,17 @@ from __future__ import absolute_import import logging -import six -from six.moves.urllib.parse import urljoin +import six from django.conf import settings -from django.urls import reverse from django.http import HttpResponse from django.template import engines +from django.urls import reverse +from six.moves.urllib.parse import urljoin from openedx.core.djangoapps.site_configuration import helpers as configuration_helpers from openedx.core.djangoapps.theming.helpers import is_request_in_themed_site +from xmodule.util.xmodule_django import get_current_request_hostname from . import Engines @@ -70,9 +71,13 @@ def marketing_link(name): elif not enable_mktg_site and name in link_map: # don't try to reverse disabled marketing links if link_map[name] is not None: - return reverse(link_map[name]) + host_name = get_current_request_hostname() + if all([host_name and 'edge' in host_name, 'http' in link_map[name]]): + return link_map[name] + else: + return reverse(link_map[name]) else: - log.debug("Cannot find corresponding link for name: %s", name) + log.debug(u"Cannot find corresponding link for name: %s", name) return '#'