From 4922877b1f420a7a2e597df055d2173a15d1496c Mon Sep 17 00:00:00 2001 From: Josue Balandrano Coronel Date: Wed, 3 Jul 2019 17:26:36 -0500 Subject: [PATCH 01/20] feat: update enrollment serializer and add problem submission history endpoint (cherry picked from commit ba4ae79bfe5db94dfabe91baf589c7434626dba2) --- lms/djangoapps/courseware/access_utils.py | 20 +- lms/djangoapps/support/tests/test_views.py | 2 +- lms/envs/test.py | 14 + .../content/block_structure/store.py | 14 +- .../djangoapps/enrollments/serializers.py | 43 +- ...ourse-enrollments-api-list-valid-data.json | 512 +++++++++++++++++- .../enrollments/tests/test_views.py | 24 +- openedx/core/djangoapps/enrollments/urls.py | 2 + openedx/core/djangoapps/enrollments/views.py | 208 ++++++- 9 files changed, 774 insertions(+), 65 deletions(-) diff --git a/lms/djangoapps/courseware/access_utils.py b/lms/djangoapps/courseware/access_utils.py index 860f2810452e..e855a69885ec 100644 --- a/lms/djangoapps/courseware/access_utils.py +++ b/lms/djangoapps/courseware/access_utils.py @@ -9,23 +9,21 @@ from django.conf import settings from pytz import UTC +from xmodule.course_module import COURSE_VISIBILITY_PUBLIC +from xmodule.util.xmodule_django import get_current_request_hostname + +from common.djangoapps.student.models import CourseEnrollment +from common.djangoapps.student.roles import CourseBetaTesterRole from lms.djangoapps.courseware.access_response import ( AccessResponse, - StartDateError, - EnrollmentRequiredAccessError, AuthenticationRequiredAccessError, + EnrollmentRequiredAccessError, + StartDateError ) from lms.djangoapps.courseware.masquerade import get_course_masquerade, is_masquerading_as_student from openedx.core.djangoapps.util.user_messages import PageLevelMessages # lint-amnesty, pylint: disable=unused-import from openedx.core.djangolib.markup import HTML # lint-amnesty, pylint: disable=unused-import -from openedx.features.course_experience import ( - COURSE_PRE_START_ACCESS_FLAG, - COURSE_ENABLE_UNENROLLED_ACCESS_FLAG, -) -from common.djangoapps.student.models import CourseEnrollment -from common.djangoapps.student.roles import CourseBetaTesterRole -from xmodule.util.xmodule_django import get_current_request_hostname -from xmodule.course_module import COURSE_VISIBILITY_PUBLIC +from openedx.features.course_experience import COURSE_ENABLE_UNENROLLED_ACCESS_FLAG, COURSE_PRE_START_ACCESS_FLAG DEBUG_ACCESS = False log = getLogger(__name__) @@ -75,7 +73,7 @@ def check_start_date(user, days_early_for_beta, start, course_key, display_error Returns: AccessResponse: Either ACCESS_GRANTED or StartDateError. """ - start_dates_disabled = settings.FEATURES['DISABLE_START_DATES'] + start_dates_disabled = settings.FEATURES.get('DISABLE_START_DATES', False) masquerading_as_student = is_masquerading_as_student(user, course_key) if start_dates_disabled and not masquerading_as_student: diff --git a/lms/djangoapps/support/tests/test_views.py b/lms/djangoapps/support/tests/test_views.py index 7b5acda2717d..48dce9ed1e6d 100644 --- a/lms/djangoapps/support/tests/test_views.py +++ b/lms/djangoapps/support/tests/test_views.py @@ -473,7 +473,7 @@ def test_change_enrollment_mode_fullfills_entitlement(self, search_string_type, 'course_id': str(self.course.id), 'old_mode': CourseMode.AUDIT, 'new_mode': CourseMode.VERIFIED, - 'reason': 'Financial Assistance' + 'reason': u'Financial Assistance' }, content_type='application/json') entitlement.refresh_from_db() assert response.status_code == 200 diff --git a/lms/envs/test.py b/lms/envs/test.py index 69b290fc53e1..69c30a966493 100644 --- a/lms/envs/test.py +++ b/lms/envs/test.py @@ -603,3 +603,17 @@ #################### Network configuration #################### # Tests are not behind any proxies CLOSEST_CLIENT_IP_FROM_HEADERS = [] + +COURSE_ENROLLMENT_MODES['test'] = { + "id": 8, + "slug": u"test", + "display_name": u"Test", + "min_price": 0 +} + +COURSE_ENROLLMENT_MODES['test_mode'] = { + "id": 9, + "slug": u"test_mode", + "display_name": u"Test Mode", + "min_price": 0 +} diff --git a/openedx/core/djangoapps/content/block_structure/store.py b/openedx/core/djangoapps/content/block_structure/store.py index 2342079bdc6e..13688fc85e43 100644 --- a/openedx/core/djangoapps/content/block_structure/store.py +++ b/openedx/core/djangoapps/content/block_structure/store.py @@ -228,12 +228,14 @@ def _encode_root_cache_key(bs_model): Returns the cache key to use for the given BlockStructureModel or StubModel. """ - if config.STORAGE_BACKING_FOR_CACHE.is_enabled(): - return str(bs_model) - return "v{version}.root.key.{root_usage_key}".format( - version=str(BlockStructureBlockData.VERSION), - root_usage_key=str(bs_model.data_usage_key), - ) + if _is_storage_backing_enabled(): + return six.text_type(bs_model) + + else: + return u"v{version}.root.key.{root_usage_key}".format( + version=six.text_type(BlockStructureBlockData.VERSION), + root_usage_key=six.text_type(bs_model.data_usage_key), + ) @staticmethod def _version_data_of_block(root_block): diff --git a/openedx/core/djangoapps/enrollments/serializers.py b/openedx/core/djangoapps/enrollments/serializers.py index 9fde7c04033a..a59cf98b988a 100644 --- a/openedx/core/djangoapps/enrollments/serializers.py +++ b/openedx/core/djangoapps/enrollments/serializers.py @@ -5,10 +5,13 @@ import logging +from django.core.exceptions import PermissionDenied from rest_framework import serializers +from xmodule.modulestore.django import modulestore from common.djangoapps.course_modes.models import CourseMode from common.djangoapps.student.models import CourseEnrollment +from lms.djangoapps.grades.course_grade_factory import CourseGradeFactory log = logging.getLogger(__name__) @@ -83,15 +86,49 @@ class CourseEnrollmentSerializer(serializers.ModelSerializer): """ course_details = CourseSerializer(source="course_overview") - user = serializers.SerializerMethodField('get_username') + user = serializers.SerializerMethodField("get_username") + finished = serializers.SerializerMethodField() + grading = serializers.SerializerMethodField() def get_username(self, model): """Retrieves the username from the associated model.""" return model.username - class Meta: + def get_finished(self, model): + """Retrieve finished course.""" + course = modulestore().get_course(model.course_id) + if course: + try: + coursegrade = CourseGradeFactory().read(model.user, course).passed + except PermissionDenied: + return False + return coursegrade + return False + + def get_grading(self, model): + """Retrieve course grade.""" + course = modulestore().get_course(model.course_id) + course_grade = None + summary = [] + current_grade = 0 + if course: + try: + course_grade = CourseGradeFactory().read(model.user, course) + current_grade = int(course_grade.percent * 100) + for section in course_grade.summary.get(u'section_breakdown'): + if section.get(u'prominent'): + summary.append(section) + except PermissionDenied: + pass + return [ + {u'current_grade': current_grade, + u'certificate_eligible': course_grade.passed if course_grade else False, + u'summary': summary} + ] + + class Meta(object): model = CourseEnrollment - fields = ('created', 'mode', 'is_active', 'course_details', 'user') + fields = ('created', 'mode', 'is_active', 'course_details', 'user', 'finished', 'grading') lookup_field = 'username' diff --git a/openedx/core/djangoapps/enrollments/tests/fixtures/course-enrollments-api-list-valid-data.json b/openedx/core/djangoapps/enrollments/tests/fixtures/course-enrollments-api-list-valid-data.json index e9fd2f55eca7..60324c0f1555 100644 --- a/openedx/core/djangoapps/enrollments/tests/fixtures/course-enrollments-api-list-valid-data.json +++ b/openedx/core/djangoapps/enrollments/tests/fixtures/course-enrollments-api-list-valid-data.json @@ -9,14 +9,74 @@ "is_active": true, "mode": "honor", "user": "student1", - "created": "2018-01-01T00:00:01Z" + "created": "2018-01-01T00:00:01Z", + "finished": false, + "grading": [{ + "certificate_eligible": false, + "current_grade": 0, + "summary": [{ + "category": "Homework", + "prominent": true, + "percent": 0.0, + "detail": "Homework Average = 0%", + "label": "HW Avg" + },{ + "category": "Lab", + "prominent": true, + "percent": 0.0, + "detail": "Lab Average = 0%", + "label": "Lab Avg" + },{ + "category": "Midterm Exam", + "prominent": true, + "percent": 0.0, + "detail": "Midterm Exam = 0%", + "label": "Midterm" + },{ + "category": "Final Exam", + "prominent": true, + "percent": 0.0, + "detail": "Final Exam = 0%", + "label": "Final" + }] + }] }, { "course_id": "e/d/X", "is_active": true, "mode": "honor", "user": "student2", - "created": "2018-01-01T00:00:01Z" + "created": "2018-01-01T00:00:01Z", + "finished": false, + "grading": [{ + "certificate_eligible": false, + "current_grade": 0, + "summary": [{ + "category": "Homework", + "prominent": true, + "percent": 0.0, + "detail": "Homework Average = 0%", + "label": "HW Avg" + },{ + "category": "Lab", + "prominent": true, + "percent": 0.0, + "detail": "Lab Average = 0%", + "label": "Lab Avg" + },{ + "category": "Midterm Exam", + "prominent": true, + "percent": 0.0, + "detail": "Midterm Exam = 0%", + "label": "Midterm" + },{ + "category": "Final Exam", + "prominent": true, + "percent": 0.0, + "detail": "Final Exam = 0%", + "label": "Final" + }] + }] } ] ], @@ -30,21 +90,111 @@ "is_active": true, "mode": "verified", "user": "staff", - "created": "2018-01-01T00:00:01Z" + "created": "2018-01-01T00:00:01Z", + "finished": false, + "grading": [{ + "certificate_eligible": false, + "current_grade": 0, + "summary": [{ + "category": "Homework", + "prominent": true, + "percent": 0.0, + "detail": "Homework Average = 0%", + "label": "HW Avg" + },{ + "category": "Lab", + "prominent": true, + "percent": 0.0, + "detail": "Lab Average = 0%", + "label": "Lab Avg" + },{ + "category": "Midterm Exam", + "prominent": true, + "percent": 0.0, + "detail": "Midterm Exam = 0%", + "label": "Midterm" + },{ + "category": "Final Exam", + "prominent": true, + "percent": 0.0, + "detail": "Final Exam = 0%", + "label": "Final" + }] + }] }, { "course_id": "x/y/Z", "is_active": true, "mode": "honor", "user": "student2", - "created": "2018-01-01T00:00:01Z" + "created": "2018-01-01T00:00:01Z", + "finished": false, + "grading": [{ + "certificate_eligible": false, + "current_grade": 0, + "summary": [{ + "category": "Homework", + "prominent": true, + "percent": 0.0, + "detail": "Homework Average = 0%", + "label": "HW Avg" + },{ + "category": "Lab", + "prominent": true, + "percent": 0.0, + "detail": "Lab Average = 0%", + "label": "Lab Avg" + },{ + "category": "Midterm Exam", + "prominent": true, + "percent": 0.0, + "detail": "Midterm Exam = 0%", + "label": "Midterm" + },{ + "category": "Final Exam", + "prominent": true, + "percent": 0.0, + "detail": "Final Exam = 0%", + "label": "Final" + }] + }] }, { "course_id": "x/y/Z", "is_active": true, "mode": "verified", "user": "student3", - "created": "2018-01-01T00:00:01Z" + "created": "2018-01-01T00:00:01Z", + "finished": false, + "grading": [{ + "certificate_eligible": false, + "current_grade": 0, + "summary": [{ + "category": "Homework", + "prominent": true, + "percent": 0.0, + "detail": "Homework Average = 0%", + "label": "HW Avg" + },{ + "category": "Lab", + "prominent": true, + "percent": 0.0, + "detail": "Lab Average = 0%", + "label": "Lab Avg" + },{ + "category": "Midterm Exam", + "prominent": true, + "percent": 0.0, + "detail": "Midterm Exam = 0%", + "label": "Midterm" + },{ + "category": "Final Exam", + "prominent": true, + "percent": 0.0, + "detail": "Final Exam = 0%", + "label": "Final" + }] + }] } ] ], @@ -59,14 +209,74 @@ "is_active": true, "mode": "honor", "user": "student2", - "created": "2018-01-01T00:00:01Z" + "created": "2018-01-01T00:00:01Z", + "finished": false, + "grading": [{ + "certificate_eligible": false, + "current_grade": 0, + "summary": [{ + "category": "Homework", + "prominent": true, + "percent": 0.0, + "detail": "Homework Average = 0%", + "label": "HW Avg" + },{ + "category": "Lab", + "prominent": true, + "percent": 0.0, + "detail": "Lab Average = 0%", + "label": "Lab Avg" + },{ + "category": "Midterm Exam", + "prominent": true, + "percent": 0.0, + "detail": "Midterm Exam = 0%", + "label": "Midterm" + },{ + "category": "Final Exam", + "prominent": true, + "percent": 0.0, + "detail": "Final Exam = 0%", + "label": "Final" + }] + }] }, { "course_id": "x/y/Z", "is_active": true, "mode": "verified", "user": "student3", - "created": "2018-01-01T00:00:01Z" + "created": "2018-01-01T00:00:01Z", + "finished": false, + "grading": [{ + "certificate_eligible": false, + "current_grade": 0, + "summary": [{ + "category": "Homework", + "prominent": true, + "percent": 0.0, + "detail": "Homework Average = 0%", + "label": "HW Avg" + },{ + "category": "Lab", + "prominent": true, + "percent": 0.0, + "detail": "Lab Average = 0%", + "label": "Lab Avg" + },{ + "category": "Midterm Exam", + "prominent": true, + "percent": 0.0, + "detail": "Midterm Exam = 0%", + "label": "Midterm" + },{ + "category": "Final Exam", + "prominent": true, + "percent": 0.0, + "detail": "Final Exam = 0%", + "label": "Final" + }] + }] } ] ], @@ -81,7 +291,37 @@ "is_active": true, "mode": "honor", "user": "student2", - "created": "2018-01-01T00:00:01Z" + "created": "2018-01-01T00:00:01Z", + "finished": false, + "grading": [{ + "certificate_eligible": false, + "current_grade": 0, + "summary": [{ + "category": "Homework", + "prominent": true, + "percent": 0.0, + "detail": "Homework Average = 0%", + "label": "HW Avg" + },{ + "category": "Lab", + "prominent": true, + "percent": 0.0, + "detail": "Lab Average = 0%", + "label": "Lab Avg" + },{ + "category": "Midterm Exam", + "prominent": true, + "percent": 0.0, + "detail": "Midterm Exam = 0%", + "label": "Midterm" + },{ + "category": "Final Exam", + "prominent": true, + "percent": 0.0, + "detail": "Final Exam = 0%", + "label": "Final" + }] + }] } ] ], @@ -95,21 +335,111 @@ "is_active": true, "mode": "verified", "user": "staff", - "created": "2018-01-01T00:00:01Z" + "created": "2018-01-01T00:00:01Z", + "finished": false, + "grading": [{ + "certificate_eligible": false, + "current_grade": 0, + "summary": [{ + "category": "Homework", + "prominent": true, + "percent": 0.0, + "detail": "Homework Average = 0%", + "label": "HW Avg" + },{ + "category": "Lab", + "prominent": true, + "percent": 0.0, + "detail": "Lab Average = 0%", + "label": "Lab Avg" + },{ + "category": "Midterm Exam", + "prominent": true, + "percent": 0.0, + "detail": "Midterm Exam = 0%", + "label": "Midterm" + },{ + "category": "Final Exam", + "prominent": true, + "percent": 0.0, + "detail": "Final Exam = 0%", + "label": "Final" + }] + }] }, { "course_id": "e/d/X", "is_active": true, "mode": "honor", "user": "student2", - "created": "2018-01-01T00:00:01Z" + "created": "2018-01-01T00:00:01Z", + "finished": false, + "grading": [{ + "certificate_eligible": false, + "current_grade": 0, + "summary": [{ + "category": "Homework", + "prominent": true, + "percent": 0.0, + "detail": "Homework Average = 0%", + "label": "HW Avg" + },{ + "category": "Lab", + "prominent": true, + "percent": 0.0, + "detail": "Lab Average = 0%", + "label": "Lab Avg" + },{ + "category": "Midterm Exam", + "prominent": true, + "percent": 0.0, + "detail": "Midterm Exam = 0%", + "label": "Midterm" + },{ + "category": "Final Exam", + "prominent": true, + "percent": 0.0, + "detail": "Final Exam = 0%", + "label": "Final" + }] + }] }, { "course_id": "x/y/Z", "is_active": true, "mode": "honor", "user": "student2", - "created": "2018-01-01T00:00:01Z" + "created": "2018-01-01T00:00:01Z", + "finished": false, + "grading": [{ + "certificate_eligible": false, + "current_grade": 0, + "summary": [{ + "category": "Homework", + "prominent": true, + "percent": 0.0, + "detail": "Homework Average = 0%", + "label": "HW Avg" + },{ + "category": "Lab", + "prominent": true, + "percent": 0.0, + "detail": "Lab Average = 0%", + "label": "Lab Avg" + },{ + "category": "Midterm Exam", + "prominent": true, + "percent": 0.0, + "detail": "Midterm Exam = 0%", + "label": "Midterm" + },{ + "category": "Final Exam", + "prominent": true, + "percent": 0.0, + "detail": "Final Exam = 0%", + "label": "Final" + }] + }] } ] @@ -122,35 +452,185 @@ "is_active": true, "mode": "honor", "user": "student1", - "created": "2018-01-01T00:00:01Z" + "created": "2018-01-01T00:00:01Z", + "finished": false, + "grading": [{ + "certificate_eligible": false, + "current_grade": 0, + "summary": [{ + "category": "Homework", + "prominent": true, + "percent": 0.0, + "detail": "Homework Average = 0%", + "label": "HW Avg" + },{ + "category": "Lab", + "prominent": true, + "percent": 0.0, + "detail": "Lab Average = 0%", + "label": "Lab Avg" + },{ + "category": "Midterm Exam", + "prominent": true, + "percent": 0.0, + "detail": "Midterm Exam = 0%", + "label": "Midterm" + },{ + "category": "Final Exam", + "prominent": true, + "percent": 0.0, + "detail": "Final Exam = 0%", + "label": "Final" + }] + }] }, { "course_id": "e/d/X", "is_active": true, "mode": "honor", "user": "student2", - "created": "2018-01-01T00:00:01Z" + "created": "2018-01-01T00:00:01Z", + "finished": false, + "grading": [{ + "certificate_eligible": false, + "current_grade": 0, + "summary": [{ + "category": "Homework", + "prominent": true, + "percent": 0.0, + "detail": "Homework Average = 0%", + "label": "HW Avg" + },{ + "category": "Lab", + "prominent": true, + "percent": 0.0, + "detail": "Lab Average = 0%", + "label": "Lab Avg" + },{ + "category": "Midterm Exam", + "prominent": true, + "percent": 0.0, + "detail": "Midterm Exam = 0%", + "label": "Midterm" + },{ + "category": "Final Exam", + "prominent": true, + "percent": 0.0, + "detail": "Final Exam = 0%", + "label": "Final" + }] + }] }, { "course_id": "x/y/Z", "is_active": true, "mode": "verified", "user": "student3", - "created": "2018-01-01T00:00:01Z" + "created": "2018-01-01T00:00:01Z", + "finished": false, + "grading": [{ + "certificate_eligible": false, + "current_grade": 0, + "summary": [{ + "category": "Homework", + "prominent": true, + "percent": 0.0, + "detail": "Homework Average = 0%", + "label": "HW Avg" + },{ + "category": "Lab", + "prominent": true, + "percent": 0.0, + "detail": "Lab Average = 0%", + "label": "Lab Avg" + },{ + "category": "Midterm Exam", + "prominent": true, + "percent": 0.0, + "detail": "Midterm Exam = 0%", + "label": "Midterm" + },{ + "category": "Final Exam", + "prominent": true, + "percent": 0.0, + "detail": "Final Exam = 0%", + "label": "Final" + }] + }] }, { "course_id": "x/y/Z", "is_active": true, "mode": "honor", "user": "student2", - "created": "2018-01-01T00:00:01Z" + "created": "2018-01-01T00:00:01Z", + "finished": false, + "grading": [{ + "certificate_eligible": false, + "current_grade": 0, + "summary": [{ + "category": "Homework", + "prominent": true, + "percent": 0.0, + "detail": "Homework Average = 0%", + "label": "HW Avg" + },{ + "category": "Lab", + "prominent": true, + "percent": 0.0, + "detail": "Lab Average = 0%", + "label": "Lab Avg" + },{ + "category": "Midterm Exam", + "prominent": true, + "percent": 0.0, + "detail": "Midterm Exam = 0%", + "label": "Midterm" + },{ + "category": "Final Exam", + "prominent": true, + "percent": 0.0, + "detail": "Final Exam = 0%", + "label": "Final" + }] + }] }, { "course_id": "x/y/Z", "is_active": true, "mode": "verified", "user": "staff", - "created": "2018-01-01T00:00:01Z" + "created": "2018-01-01T00:00:01Z", + "finished": false, + "grading": [{ + "certificate_eligible": false, + "current_grade": 0, + "summary": [{ + "category": "Homework", + "prominent": true, + "percent": 0.0, + "detail": "Homework Average = 0%", + "label": "HW Avg" + },{ + "category": "Lab", + "prominent": true, + "percent": 0.0, + "detail": "Lab Average = 0%", + "label": "Lab Avg" + },{ + "category": "Midterm Exam", + "prominent": true, + "percent": 0.0, + "detail": "Midterm Exam = 0%", + "label": "Midterm" + },{ + "category": "Final Exam", + "prominent": true, + "percent": 0.0, + "detail": "Final Exam = 0%", + "label": "Final" + }] + }] } ] ] diff --git a/openedx/core/djangoapps/enrollments/tests/test_views.py b/openedx/core/djangoapps/enrollments/tests/test_views.py index a55f2a776058..88e2d6401d8c 100644 --- a/openedx/core/djangoapps/enrollments/tests/test_views.py +++ b/openedx/core/djangoapps/enrollments/tests/test_views.py @@ -9,9 +9,10 @@ import json import unittest from unittest.mock import patch -import pytest + import ddt import httpretty +import pytest import pytz from django.conf import settings from django.core.cache import cache @@ -23,9 +24,16 @@ from freezegun import freeze_time from rest_framework import status from rest_framework.test import APITestCase +from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase +from xmodule.modulestore.tests.factories import CourseFactory, check_mongo_calls_range from common.djangoapps.course_modes.models import CourseMode from common.djangoapps.course_modes.tests.factories import CourseModeFactory +from common.djangoapps.student.models import CourseEnrollment +from common.djangoapps.student.roles import CourseStaffRole +from common.djangoapps.student.tests.factories import AdminFactory, SuperuserFactory, UserFactory +from common.djangoapps.util.models import RateLimitConfiguration +from common.djangoapps.util.testing import UrlResetMixin from openedx.core.djangoapps.content.course_overviews.models import CourseOverview from openedx.core.djangoapps.course_groups import cohorts from openedx.core.djangoapps.embargo.models import Country, CountryAccessRule, RestrictedCourse @@ -38,13 +46,6 @@ from openedx.core.lib.django_test_client_utils import get_absolute_url from openedx.features.enterprise_support.tests import FAKE_ENTERPRISE_CUSTOMER from openedx.features.enterprise_support.tests.mixins.enterprise import EnterpriseServiceMockMixin -from common.djangoapps.student.models import CourseEnrollment -from common.djangoapps.student.roles import CourseStaffRole -from common.djangoapps.student.tests.factories import AdminFactory, SuperuserFactory, UserFactory -from common.djangoapps.util.models import RateLimitConfiguration -from common.djangoapps.util.testing import UrlResetMixin -from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase -from xmodule.modulestore.tests.factories import CourseFactory, check_mongo_calls_range class EnrollmentTestMixin: @@ -62,7 +63,7 @@ def assert_enrollment_status( is_active=None, enrollment_attributes=None, min_mongo_calls=0, - max_mongo_calls=0, + max_mongo_calls=8, linked_enterprise_customer=None, cohort=None, ): @@ -378,10 +379,7 @@ def test_enrollment_list_permissions(self): mode_slug=CourseMode.DEFAULT_MODE_SLUG, mode_display_name=CourseMode.DEFAULT_MODE_SLUG, ) - self.assert_enrollment_status( - course_id=str(course.id), - max_mongo_calls=0, - ) + self.assert_enrollment_status(course_id=six.text_type(course.id)) # Verify the user himself can see both of his enrollments. self._assert_enrollments_visible_in_list([self.course, other_course]) # Verify that self.other_user can't see any of the enrollments. diff --git a/openedx/core/djangoapps/enrollments/urls.py b/openedx/core/djangoapps/enrollments/urls.py index f50221bae005..e45f5a5cb851 100644 --- a/openedx/core/djangoapps/enrollments/urls.py +++ b/openedx/core/djangoapps/enrollments/urls.py @@ -13,6 +13,7 @@ EnrollmentListView, EnrollmentUserRolesView, EnrollmentView, + SubmissionHistoryView, UnenrollmentView ) @@ -29,4 +30,5 @@ EnrollmentCourseDetailView.as_view(), name='courseenrollmentdetails'), url(r'^unenroll/$', UnenrollmentView.as_view(), name='unenrollment'), url(r'^roles/$', EnrollmentUserRolesView.as_view(), name='roles'), + url(r'^submission_history$', SubmissionHistoryView.as_view(), name='submissionhistory'), ] diff --git a/openedx/core/djangoapps/enrollments/views.py b/openedx/core/djangoapps/enrollments/views.py index e1616e23e2a5..f0e48e2ef903 100644 --- a/openedx/core/djangoapps/enrollments/views.py +++ b/openedx/core/djangoapps/enrollments/views.py @@ -5,22 +5,45 @@ """ +import json import logging -from common.djangoapps.course_modes.models import CourseMode -from django.core.exceptions import ObjectDoesNotExist, ValidationError # lint-amnesty, pylint: disable=wrong-import-order +from course_modes.models import CourseMode +from django.core.exceptions import ( # lint-amnesty, pylint: disable=wrong-import-order + ObjectDoesNotExist, + ValidationError +) from django.utils.decorators import method_decorator # lint-amnesty, pylint: disable=wrong-import-order -from edx_rest_framework_extensions.auth.jwt.authentication import JwtAuthentication # lint-amnesty, pylint: disable=wrong-import-order -from edx_rest_framework_extensions.auth.session.authentication import SessionAuthenticationAllowInactiveUser # lint-amnesty, pylint: disable=wrong-import-order +from edx_rest_framework_extensions.auth.jwt.authentication import \ + JwtAuthentication # lint-amnesty, pylint: disable=wrong-import-order +from edx_rest_framework_extensions.auth.session.authentication import \ + SessionAuthenticationAllowInactiveUser # lint-amnesty, pylint: disable=wrong-import-order from opaque_keys import InvalidKeyError # lint-amnesty, pylint: disable=wrong-import-order from opaque_keys.edx.keys import CourseKey # lint-amnesty, pylint: disable=wrong-import-order +from opaque_keys.edx.locator import CourseLocator +from rest_framework import permissions, status # lint-amnesty, pylint: disable=wrong-import-order +from rest_framework.generics import ListAPIView # lint-amnesty, pylint: disable=wrong-import-order +from rest_framework.response import Response # lint-amnesty, pylint: disable=wrong-import-order +from rest_framework.throttling import UserRateThrottle # lint-amnesty, pylint: disable=wrong-import-order +from rest_framework.views import APIView # lint-amnesty, pylint: disable=wrong-import-order +from six import text_type + +from common.djangoapps.course_modes.models import CourseMode +from common.djangoapps.student.auth import user_has_role +from common.djangoapps.student.models import CourseEnrollment, User +from common.djangoapps.student.roles import CourseStaffRole, GlobalStaff +from common.djangoapps.util.disable_rate_limit import can_disable_rate_limit +from lms.djangoapps.courseware.courses import get_course +from lms.djangoapps.courseware.models import BaseStudentModuleHistory, StudentModule from openedx.core.djangoapps.cors_csrf.authentication import SessionAuthenticationCrossDomainCsrf from openedx.core.djangoapps.cors_csrf.decorators import ensure_csrf_cookie_cross_domain from openedx.core.djangoapps.course_groups.cohorts import CourseUserGroup, add_user_to_cohort, get_cohort_by_name from openedx.core.djangoapps.embargo import api as embargo_api from openedx.core.djangoapps.enrollments import api from openedx.core.djangoapps.enrollments.errors import ( - CourseEnrollmentError, CourseEnrollmentExistsError, CourseModeNotFoundError, + CourseEnrollmentError, + CourseEnrollmentExistsError, + CourseModeNotFoundError ) from openedx.core.djangoapps.enrollments.forms import CourseEnrollmentsApiListForm from openedx.core.djangoapps.enrollments.paginators import CourseEnrollmentsApiListPagination @@ -28,7 +51,10 @@ from openedx.core.djangoapps.user_api.accounts.permissions import CanRetireUser from openedx.core.djangoapps.user_api.models import UserRetirementStatus from openedx.core.djangoapps.user_api.preferences.api import update_email_opt_in -from openedx.core.lib.api.authentication import BearerAuthenticationAllowInactiveUser +from openedx.core.lib.api.authentication import ( + BearerAuthenticationAllowInactiveUser, + OAuth2AuthenticationAllowInactiveUser +) from openedx.core.lib.api.permissions import ApiKeyHeaderPermission, ApiKeyHeaderPermissionIsAuthenticated from openedx.core.lib.api.view_utils import DeveloperErrorViewMixin from openedx.core.lib.exceptions import CourseNotFoundError @@ -39,15 +65,6 @@ EnterpriseApiServiceClient, enterprise_enabled ) -from rest_framework import permissions, status # lint-amnesty, pylint: disable=wrong-import-order -from rest_framework.generics import ListAPIView # lint-amnesty, pylint: disable=wrong-import-order -from rest_framework.response import Response # lint-amnesty, pylint: disable=wrong-import-order -from rest_framework.throttling import UserRateThrottle # lint-amnesty, pylint: disable=wrong-import-order -from rest_framework.views import APIView # lint-amnesty, pylint: disable=wrong-import-order -from common.djangoapps.student.auth import user_has_role -from common.djangoapps.student.models import CourseEnrollment, User -from common.djangoapps.student.roles import CourseStaffRole, GlobalStaff -from common.djangoapps.util.disable_rate_limit import can_disable_rate_limit log = logging.getLogger(__name__) REQUIRED_ATTRIBUTES = { @@ -965,3 +982,164 @@ def get_queryset(self): if usernames: queryset = queryset.filter(user__username__in=usernames) return queryset + + +@can_disable_rate_limit +class SubmissionHistoryView(APIView, ApiKeyPermissionMixIn): + """ + Submission history view. + """ + authentication_classes = (OAuth2AuthenticationAllowInactiveUser, EnrollmentCrossDomainSessionAuth) + permission_classes = (ApiKeyHeaderPermissionIsAuthenticated, ) + + def get(self, request): + """ + Get submission history details. + + **Usecases**: + + Regular users can only retrieve their own submission history and users with GlobalStaff status + can retrieve everyone's submission history. + + **Example Requests**: + + GET /api/enrollment/v1/submission_history?course_id=course_id + GET /api/enrollment/v1/submission_history?course_id=course_id&user=username + GET /api/enrollment/v1/submission_history?course_id=course_id&all_users=true + + **Query Parameters for GET** + + * course_id: Course id to retrieve submission history. + * username: Single username for which this view will retrieve the submission history details. + If no username specified the requester's username will be used. + * all_users: If true and if the requester has the correct permissions, + retrieve history submission from every user in a course id. + + **Response Values**: + + If there's an error while getting the submission history an empty response will + be returned. + The submission history response has the following attributes: + + * Results: A list of submission history: + * course_id: Course id + * course_name: Course name + * user: Username + * problems: List of problems + * location: problem location + * name: problem's display name + * submission_history: List of submission history + * state: State of submission. + * grade: Grade. + * max_grade: Maximum possible grade. + * data: problem's data. + """ + username = request.GET.get('username', request.user.username) + data = [] + if GlobalStaff().has_user(request.user): + all_users = bool(request.GET.get('all', False)) + else: + all_users = False + course_id = request.GET.get('course_id') + + if not (all_users or username == request.user.username or GlobalStaff().has_user(request.user) or + self.has_api_key_permissions(request)): + return Response(data) + + course_enrollments = CourseEnrollment.objects.select_related('user').filter(is_active=True) + if course_id: + if not course_id.startswith("course-v1:"): + course_id = "course-v1:{}".format(course_id) + try: + course_enrollments = course_enrollments.filter( + course_id=CourseLocator.from_string(course_id.replace(' ', '+')) + ).order_by('created') + except KeyError: + return Response(data) + + if not all_users: + course_enrollments = course_enrollments.filter(user__username=username).order_by('created') + + courses = {} + for course_enrollment in course_enrollments: + try: + course_list = courses.get(course_enrollment.course_id) + if course_list: + course, course_children = course_list + else: + course = get_course(course_enrollment.course_id, depth=4) + course_children = course.get_children() + courses[course_enrollment.course_id] = [course, course_children] + except ValueError: + continue + course_data = self._get_course_data(course_enrollment, course, course_children) + data.append(course_data) + + return Response({'results': data}) + + def _get_problem_data(self, course_enrollment, component): + """ + Get problem data from a course enrollment. + + Args: + ----- + course_enrollment: Course Enrollment. + component: Component to analyze. + """ + problem_data = { + 'location': str(component.location), + 'name': component.display_name, + 'submission_history': [], + 'data': component.data + } + + csm = StudentModule.objects.filter( + module_state_key=component.location, + student__username=course_enrollment.user.username, + course_id=course_enrollment.course_id) + + scores = BaseStudentModuleHistory.get_history(csm) + for i, score in enumerate(scores): + if i % 2 == 1: + continue + + state = score.state + if state is not None: + state = json.loads(state) + + history_data = { + 'state': state, + 'grade': score.grade, + 'max_grade': score.max_grade + } + problem_data['submission_history'].append(history_data) + + return problem_data + + def _get_course_data(self, course_enrollment, course, course_children): + """ + Get course data. + + Params: + -------- + + course_enrollment (CourseEnrollment): course enrollment + course: course + course_children: course children + """ + + course_data = { + 'course_id': str(course_enrollment.course_id), + 'course_name': course.display_name_with_default, + 'user': course_enrollment.user.username, + 'problems': [] + } + for section in course_children: + for subsection in section.get_children(): + for vertical in subsection.get_children(): + for component in vertical.get_children(): + if component.location.category == 'problem' and getattr(component, 'has_score', False): + problem_data = self._get_problem_data(course_enrollment, component) + course_data['problems'].append(problem_data) + + return course_data From 5e1453196f0cb1eec897f534a7fbd0014cf372ef Mon Sep 17 00:00:00 2001 From: Matjaz Gregoric Date: Mon, 23 Aug 2021 16:38:01 +0200 Subject: [PATCH 02/20] fix: monkey-patch django db introspection to avoid performance issues (cherry picked from commit a26bb857fd7b03c797534d93fe2bf26b9b8698d2) --- cms/__init__.py | 26 ++++++++++++++++++++++++++ lms/__init__.py | 26 ++++++++++++++++++++++++++ 2 files changed, 52 insertions(+) diff --git a/cms/__init__.py b/cms/__init__.py index f9ed0bb3cea1..ff0885c93292 100644 --- a/cms/__init__.py +++ b/cms/__init__.py @@ -22,3 +22,29 @@ # that shared_task will use this app, and also ensures that the celery # singleton is always configured for the CMS. from .celery import APP as CELERY_APP # lint-amnesty, pylint: disable=wrong-import-position + +# FAL-2248: Monkey patch django's get_storage_engine to work around long migrations times. +# This fixes a performance issue with database migrations in Ocim. We will need to keep +# this patch in our opencraft-release/* branches until edx-platform upgrades to Django 4.* +# which will include this commit: +# https://github.com/django/django/commit/518ce7a51f994fc0585d31c4553e2072bf816f76 +import django.db.backends.mysql.introspection + +def get_storage_engine(self, cursor, table_name): + """ + This is a patched version of `get_storage_engine` that fixes a + performance issue with migrations. For more info see FAL-2248 and + https://github.com/django/django/pull/14766 + """ + cursor.execute(""" + SELECT engine + FROM information_schema.tables + WHERE table_name = %s + AND table_schema = DATABASE()""", [table_name]) + result = cursor.fetchone() + if not result: + return self.connection.features._mysql_storage_engine # pylint: disable=protected-access + return result[0] + + +django.db.backends.mysql.introspection.DatabaseIntrospection.get_storage_engine = get_storage_engine diff --git a/lms/__init__.py b/lms/__init__.py index 008640ac7147..3444b737f50f 100644 --- a/lms/__init__.py +++ b/lms/__init__.py @@ -18,3 +18,29 @@ # that shared_task will use this app, and also ensures that the celery # singleton is always configured for the LMS. from .celery import APP as CELERY_APP # lint-amnesty, pylint: disable=wrong-import-position + +# FAL-2248: Monkey patch django's get_storage_engine to work around long migrations times. +# This fixes a performance issue with database migrations in Ocim. We will need to keep +# this patch in our opencraft-release/* branches until edx-platform upgrades to Django 4.* +# which will include this commit: +# https://github.com/django/django/commit/518ce7a51f994fc0585d31c4553e2072bf816f76 +import django.db.backends.mysql.introspection + +def get_storage_engine(self, cursor, table_name): + """ + This is a patched version of `get_storage_engine` that fixes a + performance issue with migrations. For more info see FAL-2248 and + https://github.com/django/django/pull/14766 + """ + cursor.execute(""" + SELECT engine + FROM information_schema.tables + WHERE table_name = %s + AND table_schema = DATABASE()""", [table_name]) + result = cursor.fetchone() + if not result: + return self.connection.features._mysql_storage_engine # pylint: disable=protected-access + return result[0] + + +django.db.backends.mysql.introspection.DatabaseIntrospection.get_storage_engine = get_storage_engine From f5a95b0d3df0e0a203ef3b7dd03933e25006dab6 Mon Sep 17 00:00:00 2001 From: ha-D Date: Thu, 26 Aug 2021 23:13:34 +0000 Subject: [PATCH 03/20] fixup! feat: options for excluding courses from search --- cms/djangoapps/contentstore/courseware_index.py | 2 ++ lms/envs/common.py | 17 ++++++++++++++++- lms/envs/production.py | 9 +++++++++ .../courseware_search/lms_filter_generator.py | 6 ++++++ 4 files changed, 33 insertions(+), 1 deletion(-) diff --git a/cms/djangoapps/contentstore/courseware_index.py b/cms/djangoapps/contentstore/courseware_index.py index 5d9b627ec02f..1878e04b3b01 100644 --- a/cms/djangoapps/contentstore/courseware_index.py +++ b/cms/djangoapps/contentstore/courseware_index.py @@ -587,6 +587,8 @@ class CourseAboutSearchIndexer(CoursewareSearchIndexer): AboutInfo("org", AboutInfo.PROPERTY, AboutInfo.FROM_COURSE_PROPERTY), AboutInfo("modes", AboutInfo.PROPERTY, AboutInfo.FROM_COURSE_MODE), AboutInfo("language", AboutInfo.PROPERTY, AboutInfo.FROM_COURSE_PROPERTY), + AboutInfo("invitation_only", AboutInfo.PROPERTY, AboutInfo.FROM_COURSE_PROPERTY), + AboutInfo("catalog_visibility", AboutInfo.PROPERTY, AboutInfo.FROM_COURSE_PROPERTY), ] @classmethod diff --git a/lms/envs/common.py b/lms/envs/common.py index bfd5df27bddd..5bfb4ba5e192 100644 --- a/lms/envs/common.py +++ b/lms/envs/common.py @@ -817,7 +817,7 @@ # .. toggle_tickets: https://openedx.atlassian.net/browse/OSPR-1880 'ENABLE_HTML_XBLOCK_STUDENT_VIEW_DATA': False, - # .. toggle_name: FEATURES['ENABLE_CHANGE_USER_PASSWORD_ADMIN'] + # .. toggle_name: FEATURES['ENABLE_PASSWORD_RESET_FAILURE_EMAIL'] # .. toggle_implementation: DjangoSetting # .. toggle_default: False # .. toggle_description: Whether to send an email for failed password reset attempts or not. This happens when a @@ -3994,6 +3994,21 @@ def _make_locale_paths(settings): # pylint: disable=missing-function-docstring SEARCH_FILTER_GENERATOR = "lms.lib.courseware_search.lms_filter_generator.LmsSearchFilterGenerator" # Override to skip enrollment start date filtering in course search SEARCH_SKIP_ENROLLMENT_START_DATE_FILTERING = False +# .. toggle_name: SEARCH_SKIP_INVITATION_ONLY_FILTERING +# .. toggle_implementation: DjangoSetting +# .. toggle_default: True +# .. toggle_description: If enabled, invitation-only courses will appear in search results. +# .. toggle_use_cases: open_edx +# .. toggle_creation_date: 2021-08-27 +SEARCH_SKIP_INVITATION_ONLY_FILTERING = True +# .. toggle_name: SEARCH_SKIP_SHOW_IN_CATALOG_FILTERING +# .. toggle_implementation: DjangoSetting +# .. toggle_default: True +# .. toggle_description: If enabled, courses with a catalog_visibility set to "none" will still +# appear in search results. +# .. toggle_use_cases: open_edx +# .. toggle_creation_date: 2021-08-27 +SEARCH_SKIP_SHOW_IN_CATALOG_FILTERING = True # The configuration visibility of account fields. ACCOUNT_VISIBILITY_CONFIGURATION = { diff --git a/lms/envs/production.py b/lms/envs/production.py index d517908c0608..89cb7f2b4196 100644 --- a/lms/envs/production.py +++ b/lms/envs/production.py @@ -729,6 +729,15 @@ def get_env_setting(setting): SEARCH_ENGINE = "search.elastic.ElasticSearchEngine" SEARCH_FILTER_GENERATOR = ENV_TOKENS.get('SEARCH_FILTER_GENERATOR', SEARCH_FILTER_GENERATOR) +SEARCH_SKIP_INVITATION_ONLY_FILTERING = ENV_TOKENS.get( + 'SEARCH_SKIP_INVITATION_ONLY_FILTERING', + SEARCH_SKIP_INVITATION_ONLY_FILTERING, +) +SEARCH_SKIP_SHOW_IN_CATALOG_FILTERING = ENV_TOKENS.get( + 'SEARCH_SKIP_SHOW_IN_CATALOG_FILTERING', + SEARCH_SKIP_SHOW_IN_CATALOG_FILTERING, +) + # TODO: Once we have successfully upgraded to ES7, switch this back to ELASTIC_SEARCH_CONFIG. ELASTIC_SEARCH_CONFIG = ENV_TOKENS.get('ELASTIC_SEARCH_CONFIG_ES7', [{}]) diff --git a/lms/lib/courseware_search/lms_filter_generator.py b/lms/lib/courseware_search/lms_filter_generator.py index 14b539b4291e..c4e5ab7ac736 100644 --- a/lms/lib/courseware_search/lms_filter_generator.py +++ b/lms/lib/courseware_search/lms_filter_generator.py @@ -2,6 +2,7 @@ This file contains implementation override of SearchFilterGenerator which will allow * Filter by all courses in which the user is enrolled in """ +from django.conf import settings from search.filter_generator import SearchFilterGenerator from openedx.core.djangoapps.course_groups.partition_scheme import CohortPartitionScheme @@ -52,4 +53,9 @@ def exclude_dictionary(self, **kwargs): if org_filter_out_set: exclude_dictionary['org'] = list(org_filter_out_set) + if not getattr(settings, "SEARCH_SKIP_INVITATION_ONLY_FILTERING", True): + exclude_dictionary['invitation_only'] = True + if not getattr(settings, "SEARCH_SKIP_SHOW_IN_CATALOG_FILTERING", True): + exclude_dictionary['catalog_visibility'] = 'none' + return exclude_dictionary From ae9326d0118564763eaa742bb3c6386814bc1a2b Mon Sep 17 00:00:00 2001 From: Kaustav Banerjee Date: Thu, 23 Dec 2021 07:18:58 +0530 Subject: [PATCH 04/20] feat: change studio schedule datetime inputs to user timezone --- cms/djangoapps/contentstore/views/course.py | 7 +++ cms/static/cms/js/spec/main.js | 1 + cms/static/js/utils/date_utils.js | 61 ++++++++++++++++++++- 3 files changed, 67 insertions(+), 2 deletions(-) diff --git a/cms/djangoapps/contentstore/views/course.py b/cms/djangoapps/contentstore/views/course.py index ed8e09f8c886..f559a0b854c1 100644 --- a/cms/djangoapps/contentstore/views/course.py +++ b/cms/djangoapps/contentstore/views/course.py @@ -67,6 +67,7 @@ from openedx.core.djangoapps.credit.tasks import update_credit_course_requirements from openedx.core.djangoapps.models.course_details import CourseDetails from openedx.core.djangoapps.site_configuration import helpers as configuration_helpers +from openedx.core.djangoapps.user_api.models import UserPreference from openedx.core.djangolib.js_utils import dump_js_escaped_json from openedx.core.lib.course_tabs import CourseTabPluginManager from openedx.core.lib.courses import course_image_url @@ -1215,6 +1216,12 @@ def settings_handler(request, course_key_string): # lint-amnesty, pylint: disab elif 'application/json' in request.META.get('HTTP_ACCEPT', ''): if request.method == 'GET': course_details = CourseDetails.fetch(course_key) + + # Fetch the prefered timezone setup by the user + # and pass it as part of Json response + user_timezone = UserPreference.get_value(request.user, 'time_zone') + course_details.user_timezone = user_timezone + return JsonResponse( course_details, # encoder serializes dates, old locations, and instances diff --git a/cms/static/cms/js/spec/main.js b/cms/static/cms/js/spec/main.js index f5aa089b0438..384eb7b83350 100644 --- a/cms/static/cms/js/spec/main.js +++ b/cms/static/cms/js/spec/main.js @@ -47,6 +47,7 @@ 'jquery.simulate': 'xmodule_js/common_static/js/vendor/jquery.simulate', 'datepair': 'xmodule_js/common_static/js/vendor/timepicker/datepair', 'date': 'xmodule_js/common_static/js/vendor/date', + 'moment-timezone': 'common/js/vendor/moment-timezone-with-data', moment: 'common/js/vendor/moment-with-locales', 'text': 'xmodule_js/common_static/js/vendor/requirejs/text', 'underscore': 'common/js/vendor/underscore', diff --git a/cms/static/js/utils/date_utils.js b/cms/static/js/utils/date_utils.js index 0c91e6347e72..26e2c52e86cf 100644 --- a/cms/static/js/utils/date_utils.js +++ b/cms/static/js/utils/date_utils.js @@ -1,5 +1,5 @@ -define(['jquery', 'date', 'js/utils/change_on_enter', 'jquery.ui', 'jquery.timepicker'], -function($, date, TriggerChangeEventOnEnter) { +define(['jquery', 'date', 'js/utils/change_on_enter', 'moment-timezone', 'jquery.ui', 'jquery.timepicker'], +function($, date, TriggerChangeEventOnEnter, moment) { 'use strict'; function getDate(datepickerInput, timepickerInput) { @@ -67,14 +67,54 @@ function($, date, TriggerChangeEventOnEnter) { return obj; } + /** + * Calculates the utc offset in miliseconds for given + * timezone and subtracts it from given localized time + * to get time in UTC + * + * @param {Date} localTime JS Date object in Local Time + * @param {string} timezone IANA timezone name ex. "Australia/Brisbane" + * @returns JS Date object in UTC + */ + function convertLocalizedDateToUTC(localTime, timezone) { + const localTimeMS = localTime.getTime(); + const utcOffset = moment.tz(localTime, timezone)._offset; + return new Date(localTimeMS - (utcOffset * 60 *1000)); + } + + /** + * Returns the timezone abbreviation for given + * timezone name + * + * @param {string} timezone IANA timezone name ex. "Australia/Brisbane" + * @returns Timezone abbreviation ex. "AEST" + */ + function getTZAbbreviation(timezone) { + return moment(new Date()).tz(timezone).format('z'); + } + + /** + * Converts the given datetime string from UTC to localized time + * + * @param {string} utcDateTime JS Date object with UTC datetime + * @param {string} timezone IANA timezone name ex. "Australia/Brisbane" + * @returns Formatted datetime string with localized timezone + */ + function getLocalizedCurrentDate(utcDateTime, timezone) { + const localDateTime = moment(utcDateTime).tz(timezone); + return localDateTime.format('YYYY-MM-DDTHH[:]mm[:]ss'); + } + function setupDatePicker(fieldName, view, index) { var cacheModel; var div; var datefield; var timefield; + var tzfield; var cacheview; var setfield; var currentDate; + var timezone; if (typeof index !== 'undefined' && view.hasOwnProperty('collection')) { cacheModel = view.collection.models[index]; div = view.$el.find('#' + view.collectionSelector(cacheModel.cid)); @@ -84,10 +124,18 @@ function($, date, TriggerChangeEventOnEnter) { } datefield = $(div).find('input.date'); timefield = $(div).find('input.time'); + tzfield = $(div).find('span.timezone'); cacheview = view; + + timezone = cacheModel.get('user_timezone'); + setfield = function(event) { var newVal = getDate(datefield, timefield); + if (timezone) { + newVal = convertLocalizedDateToUTC(newVal, timezone); + } + // Setting to null clears the time as well, as date and time are linked. // Note also that the validation logic prevents us from clearing the start date // (start date is required by the back end). @@ -109,8 +157,17 @@ function($, date, TriggerChangeEventOnEnter) { if (cacheModel) { currentDate = cacheModel.get(fieldName); } + + if (timezone) { + const tz = getTZAbbreviation(timezone); + $(tzfield).text("("+tz+")"); + } + // timepicker doesn't let us set null, so check that we have a time if (currentDate) { + if (timezone) { + currentDate = getLocalizedCurrentDate(currentDate, timezone); + } setDate(datefield, timefield, currentDate); } else { // but reset fields either way From 7ab8541c6b8f1ab2d843ac156ffc7a096839e5df Mon Sep 17 00:00:00 2001 From: pkulkark Date: Mon, 16 May 2022 13:14:20 +0530 Subject: [PATCH 05/20] fix: Bump edx-search version to 3.4.0 --- requirements/edx/base.txt | 2 +- requirements/edx/development.txt | 2 +- requirements/edx/testing.txt | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/requirements/edx/base.txt b/requirements/edx/base.txt index 16b8ef7004ce..34ba292e7252 100644 --- a/requirements/edx/base.txt +++ b/requirements/edx/base.txt @@ -474,7 +474,7 @@ edx-rest-api-client==5.4.0 # -r requirements/edx/base.in # edx-enterprise # edx-proctoring -edx-search==3.1.0 +edx-search==3.4.0 # via -r requirements/edx/base.in edx-sga==0.17.2 # via -r requirements/edx/base.in diff --git a/requirements/edx/development.txt b/requirements/edx/development.txt index c0e8f93bf6b5..5edd6adf77ee 100644 --- a/requirements/edx/development.txt +++ b/requirements/edx/development.txt @@ -585,7 +585,7 @@ edx-rest-api-client==5.4.0 # -r requirements/edx/testing.txt # edx-enterprise # edx-proctoring -edx-search==3.1.0 +edx-search==3.4.0 # via -r requirements/edx/testing.txt edx-sga==0.17.2 # via -r requirements/edx/testing.txt diff --git a/requirements/edx/testing.txt b/requirements/edx/testing.txt index 4153ba3bfd37..450f6cce8c95 100644 --- a/requirements/edx/testing.txt +++ b/requirements/edx/testing.txt @@ -568,7 +568,7 @@ edx-rest-api-client==5.4.0 # -r requirements/edx/base.txt # edx-enterprise # edx-proctoring -edx-search==3.1.0 +edx-search==3.4.0 # via -r requirements/edx/base.txt edx-sga==0.17.2 # via -r requirements/edx/base.txt From 41b56eaf9e36b02bc6c19b46aae497d7a96163ba Mon Sep 17 00:00:00 2001 From: Meysam Date: Mon, 18 Oct 2021 17:20:17 +0300 Subject: [PATCH 06/20] fix: display right-to-left for rtl-languages in mobile (#28861) --- lms/static/sass/_header.scss | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lms/static/sass/_header.scss b/lms/static/sass/_header.scss index aa4236fd744b..1949d557def2 100644 --- a/lms/static/sass/_header.scss +++ b/lms/static/sass/_header.scss @@ -366,7 +366,7 @@ width: 100%; padding: $baseline*0.6 $baseline; border-bottom: 1px solid theme-color('light'); - text-align: left; + @include text-align(left); cursor: pointer; &:hover, From 23c75b34eefdb656d679f1770ff958369fbcdc0a Mon Sep 17 00:00:00 2001 From: Keith Grootboom Date: Wed, 9 Feb 2022 20:30:21 +0200 Subject: [PATCH 07/20] feat: add PREPEND_LOCALE_PATHS configuration setting (#29851) edx-platform supports COMPREHENSIVE_THEME_LOCALE_PATHS setting, which appends paths to the end of LOCALE_PATHS, but there's currently no way to add additional paths to the start of the list. https://tasks.opencraft.com/browse/SE-5299 --- cms/envs/common.py | 6 ++++++ cms/envs/production.py | 6 ++++++ lms/envs/common.py | 11 ++++++++++- lms/envs/production.py | 7 +++++++ lms/envs/test.py | 2 ++ 5 files changed, 31 insertions(+), 1 deletion(-) diff --git a/cms/envs/common.py b/cms/envs/common.py index 13c1375a70f8..edba66f8b200 100644 --- a/cms/envs/common.py +++ b/cms/envs/common.py @@ -2007,6 +2007,12 @@ # "COMPREHENSIVE_THEME_LOCALE_PATHS" : ["/edx/src/edx-themes/conf/locale"]. COMPREHENSIVE_THEME_LOCALE_PATHS = [] +# .. setting_name: PREPEND_LOCALE_PATHS +# .. setting_default: [] +# .. setting_description: A list of the paths to locale directories to load first e.g. +# "PREPEND_LOCALE_PATHS" : ["/edx/my-locales/"]. +PREPEND_LOCALE_PATHS = [] + # .. setting_name: DEFAULT_SITE_THEME # .. setting_default: None # .. setting_description: See LMS annotation. diff --git a/cms/envs/production.py b/cms/envs/production.py index aefd5793bdfb..3c4acdf0f77b 100644 --- a/cms/envs/production.py +++ b/cms/envs/production.py @@ -239,6 +239,12 @@ def get_env_setting(setting): # ], COMPREHENSIVE_THEME_LOCALE_PATHS = ENV_TOKENS.get('COMPREHENSIVE_THEME_LOCALE_PATHS', []) +# PREPEND_LOCALE_PATHS contain the paths to locale directories to load first e.g. +# "PREPEND_LOCALE_PATHS" : [ +# "/edx/my-locale/" +# ], +PREPEND_LOCALE_PATHS = ENV_TOKENS.get('PREPEND_LOCALE_PATHS', []) + #Timezone overrides TIME_ZONE = ENV_TOKENS.get('CELERY_TIMEZONE', CELERY_TIMEZONE) diff --git a/lms/envs/common.py b/lms/envs/common.py index 5bfb4ba5e192..18a31ad0e3ad 100644 --- a/lms/envs/common.py +++ b/lms/envs/common.py @@ -1852,7 +1852,9 @@ def _make_mako_template_dirs(settings): # Localization strings (e.g. django.po) are under these directories def _make_locale_paths(settings): # pylint: disable=missing-function-docstring - locale_paths = [settings.REPO_ROOT + '/conf/locale'] # edx-platform/conf/locale/ + locale_paths = list(settings.PREPEND_LOCALE_PATHS) + locale_paths += [settings.REPO_ROOT + '/conf/locale'] # edx-platform/conf/locale/ + if settings.ENABLE_COMPREHENSIVE_THEMING: # Add locale paths to settings for comprehensive theming. for locale_path in settings.COMPREHENSIVE_THEME_LOCALE_PATHS: @@ -4350,6 +4352,13 @@ def _make_locale_paths(settings): # pylint: disable=missing-function-docstring # "COMPREHENSIVE_THEME_LOCALE_PATHS" : ["/edx/src/edx-themes/conf/locale"]. COMPREHENSIVE_THEME_LOCALE_PATHS = [] + +# .. setting_name: PREPEND_LOCALE_PATHS +# .. setting_default: [] +# .. setting_description: A list of the paths to locale directories to load first e.g. +# "PREPEND_LOCALE_PATHS" : ["/edx/my-locales/"]. +PREPEND_LOCALE_PATHS = [] + # .. setting_name: DEFAULT_SITE_THEME # .. setting_default: None # .. setting_description: Theme to use when no site or site theme is defined, for example diff --git a/lms/envs/production.py b/lms/envs/production.py index 89cb7f2b4196..fb6eff851d3e 100644 --- a/lms/envs/production.py +++ b/lms/envs/production.py @@ -281,6 +281,13 @@ def get_env_setting(setting): COMPREHENSIVE_THEME_LOCALE_PATHS = ENV_TOKENS.get('COMPREHENSIVE_THEME_LOCALE_PATHS', []) +# PREPEND_LOCALE_PATHS contain the paths to locale directories to load first e.g. +# "PREPEND_LOCALE_PATHS" : [ +# "/edx/my-locale" +# ], +PREPEND_LOCALE_PATHS = ENV_TOKENS.get('PREPEND_LOCALE_PATHS', []) + + MKTG_URL_LINK_MAP.update(ENV_TOKENS.get('MKTG_URL_LINK_MAP', {})) ENTERPRISE_MARKETING_FOOTER_QUERY_PARAMS = ENV_TOKENS.get( 'ENTERPRISE_MARKETING_FOOTER_QUERY_PARAMS', diff --git a/lms/envs/test.py b/lms/envs/test.py index 69c30a966493..a5d8ce941e93 100644 --- a/lms/envs/test.py +++ b/lms/envs/test.py @@ -477,6 +477,8 @@ COMPREHENSIVE_THEME_LOCALE_PATHS = [REPO_ROOT / "themes/conf/locale", ] ENABLE_COMPREHENSIVE_THEMING = True +PREPEND_LOCALE_PATHS = [] + LMS_ROOT_URL = "http://localhost:8000" # Needed for derived settings used by cms only. From ee4b81c53750e64c3c47fa6f653ec4fc68d8af56 Mon Sep 17 00:00:00 2001 From: Pooja Kulkarni <13742492+pkulkark@users.noreply.github.com> Date: Thu, 31 Mar 2022 10:52:56 +0530 Subject: [PATCH 08/20] fix: Convert compliance warning back to html (#465) --- openedx/core/djangoapps/password_policy/forms.py | 5 +++-- openedx/core/djangoapps/user_authn/views/login.py | 2 +- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/openedx/core/djangoapps/password_policy/forms.py b/openedx/core/djangoapps/password_policy/forms.py index 389669c370e1..7651583053b7 100644 --- a/openedx/core/djangoapps/password_policy/forms.py +++ b/openedx/core/djangoapps/password_policy/forms.py @@ -6,6 +6,7 @@ from django.forms import ValidationError from openedx.core.djangoapps.password_policy import compliance as password_policy_compliance +from openedx.core.djangolib.markup import HTML class PasswordPolicyAwareAdminAuthForm(AdminAuthenticationForm): @@ -24,9 +25,9 @@ def clean(self): password_policy_compliance.enforce_compliance_on_login(self.user_cache, cleaned_data['password']) except password_policy_compliance.NonCompliantPasswordWarning as e: # Allow login, but warn the user that they will be required to reset their password soon. - messages.warning(self.request, str(e)) + messages.warning(self.request, HTML(str(e))) except password_policy_compliance.NonCompliantPasswordException as e: # Prevent the login attempt. - raise ValidationError(str(e)) # lint-amnesty, pylint: disable=raise-missing-from + raise ValidationError(HTML(str(e))) # lint-amnesty, pylint: disable=raise-missing-from return cleaned_data diff --git a/openedx/core/djangoapps/user_authn/views/login.py b/openedx/core/djangoapps/user_authn/views/login.py index f061c5a65419..cebafbbcf1a5 100644 --- a/openedx/core/djangoapps/user_authn/views/login.py +++ b/openedx/core/djangoapps/user_authn/views/login.py @@ -174,7 +174,7 @@ def _enforce_password_policy_compliance(request, user): # lint-amnesty, pylint: password_policy_compliance.enforce_compliance_on_login(user, request.POST.get('password')) except password_policy_compliance.NonCompliantPasswordWarning as e: # Allow login, but warn the user that they will be required to reset their password soon. - PageLevelMessages.register_warning_message(request, str(e)) + PageLevelMessages.register_warning_message(request, HTML(str(e))) except password_policy_compliance.NonCompliantPasswordException as e: # Increment the lockout counter to safguard from further brute force requests # if user's password has been compromised. From 570bdd3d5e77e7e8843dd84238478bf964480625 Mon Sep 17 00:00:00 2001 From: pkulkark Date: Fri, 20 May 2022 15:56:37 +0530 Subject: [PATCH 09/20] fix: pylint errors --- cms/__init__.py | 12 ++++++------ lms/__init__.py | 1 + .../core/djangoapps/content/block_structure/store.py | 4 +++- .../core/djangoapps/enrollments/tests/test_views.py | 1 + openedx/core/djangoapps/enrollments/views.py | 2 -- 5 files changed, 11 insertions(+), 9 deletions(-) diff --git a/cms/__init__.py b/cms/__init__.py index ff0885c93292..d1bf27534315 100644 --- a/cms/__init__.py +++ b/cms/__init__.py @@ -6,6 +6,12 @@ isort:skip_file """ +# FAL-2248: Monkey patch django's get_storage_engine to work around long migrations times. +# This fixes a performance issue with database migrations in Ocim. We will need to keep +# this patch in our opencraft-release/* branches until edx-platform upgrades to Django 4.* +# which will include this commit: +# https://github.com/django/django/commit/518ce7a51f994fc0585d31c4553e2072bf816f76 +import django.db.backends.mysql.introspection # We monkey patch Kombu's entrypoints listing because scanning through this # accounts for the majority of LMS/Studio startup time for tests, and we don't @@ -23,12 +29,6 @@ # singleton is always configured for the CMS. from .celery import APP as CELERY_APP # lint-amnesty, pylint: disable=wrong-import-position -# FAL-2248: Monkey patch django's get_storage_engine to work around long migrations times. -# This fixes a performance issue with database migrations in Ocim. We will need to keep -# this patch in our opencraft-release/* branches until edx-platform upgrades to Django 4.* -# which will include this commit: -# https://github.com/django/django/commit/518ce7a51f994fc0585d31c4553e2072bf816f76 -import django.db.backends.mysql.introspection def get_storage_engine(self, cursor, table_name): """ diff --git a/lms/__init__.py b/lms/__init__.py index 3444b737f50f..05a30f4ffad4 100644 --- a/lms/__init__.py +++ b/lms/__init__.py @@ -26,6 +26,7 @@ # https://github.com/django/django/commit/518ce7a51f994fc0585d31c4553e2072bf816f76 import django.db.backends.mysql.introspection + def get_storage_engine(self, cursor, table_name): """ This is a patched version of `get_storage_engine` that fixes a diff --git a/openedx/core/djangoapps/content/block_structure/store.py b/openedx/core/djangoapps/content/block_structure/store.py index 13688fc85e43..a1d20548b8c9 100644 --- a/openedx/core/djangoapps/content/block_structure/store.py +++ b/openedx/core/djangoapps/content/block_structure/store.py @@ -16,6 +16,8 @@ from .models import BlockStructureModel from .transformer_registry import TransformerRegistry +import six + logger = getLogger(__name__) # pylint: disable=C0103 @@ -228,7 +230,7 @@ def _encode_root_cache_key(bs_model): Returns the cache key to use for the given BlockStructureModel or StubModel. """ - if _is_storage_backing_enabled(): + if config.STORAGE_BACKING_FOR_CACHE.is_enabled(): return six.text_type(bs_model) else: diff --git a/openedx/core/djangoapps/enrollments/tests/test_views.py b/openedx/core/djangoapps/enrollments/tests/test_views.py index 88e2d6401d8c..5505461e987c 100644 --- a/openedx/core/djangoapps/enrollments/tests/test_views.py +++ b/openedx/core/djangoapps/enrollments/tests/test_views.py @@ -14,6 +14,7 @@ import httpretty import pytest import pytz +import six from django.conf import settings from django.core.cache import cache from django.core.exceptions import ImproperlyConfigured diff --git a/openedx/core/djangoapps/enrollments/views.py b/openedx/core/djangoapps/enrollments/views.py index f0e48e2ef903..ed3d803836c6 100644 --- a/openedx/core/djangoapps/enrollments/views.py +++ b/openedx/core/djangoapps/enrollments/views.py @@ -8,7 +8,6 @@ import json import logging -from course_modes.models import CourseMode from django.core.exceptions import ( # lint-amnesty, pylint: disable=wrong-import-order ObjectDoesNotExist, ValidationError @@ -26,7 +25,6 @@ from rest_framework.response import Response # lint-amnesty, pylint: disable=wrong-import-order from rest_framework.throttling import UserRateThrottle # lint-amnesty, pylint: disable=wrong-import-order from rest_framework.views import APIView # lint-amnesty, pylint: disable=wrong-import-order -from six import text_type from common.djangoapps.course_modes.models import CourseMode from common.djangoapps.student.auth import user_has_role From ca3d3e2271b30a821b61c37fb42348696e348b06 Mon Sep 17 00:00:00 2001 From: Agrendalath Date: Fri, 9 Apr 2021 00:14:53 +0200 Subject: [PATCH 10/20] feat: support adding custom editors to Studio This: 1. Introduces a variable for the Course Outline view in Studio. A custom theme can override it to add new editors. 2. Exports a function for creating new editor modals. A custom theme can use it to create editors without adding boilerplate code. 3. Adds a pluggable override for XBlock fields that are passed to the Studio. Without this, custom editors in Studio cannot retrieve values of XBlock fields. (cherry picked from commit e633cc9c249a31f308ac67036b7a62609fb29499) --- cms/djangoapps/contentstore/views/item.py | 9 +++++++++ cms/envs/common.py | 1 + cms/envs/production.py | 3 +++ cms/static/js/views/modals/course_outline_modals.js | 13 +++++++++++++ cms/static/js/views/pages/course_outline.js | 5 ++++- openedx/core/lib/xblock_utils/__init__.py | 2 +- 6 files changed, 31 insertions(+), 2 deletions(-) diff --git a/cms/djangoapps/contentstore/views/item.py b/cms/djangoapps/contentstore/views/item.py index 5036723cd3c1..0d526bef08c8 100644 --- a/cms/djangoapps/contentstore/views/item.py +++ b/cms/djangoapps/contentstore/views/item.py @@ -13,6 +13,7 @@ from django.http import Http404, HttpResponse, HttpResponseBadRequest from django.utils.translation import gettext as _ from django.views.decorators.http import require_http_methods +from edx_django_utils.plugins import pluggable_override from edx_proctoring.api import ( does_backend_support_onboarding, get_exam_by_content_id, @@ -1116,6 +1117,7 @@ def _get_gating_info(course, xblock): return info +@pluggable_override('OVERRIDE_CREATE_XBLOCK_INFO') def create_xblock_info(xblock, data=None, metadata=None, include_ancestor_info=False, include_child_info=False, # lint-amnesty, pylint: disable=too-many-statements course_outline=False, include_children_predicate=NEVER, parent_xblock=None, graders=None, user=None, course=None, is_concise=False): @@ -1134,6 +1136,13 @@ def create_xblock_info(xblock, data=None, metadata=None, include_ancestor_info=F In addition, an optional include_children_predicate argument can be provided to define whether or not a particular xblock should have its children included. + + You can customize the behavior of this function using the `OVERRIDE_CREATE_XBLOCK_INFO` pluggable override point. + For example: + >>> def create_xblock_info(default_fn, xblock, *args, **kwargs): + ... xblock_info = default_fn(xblock, *args, **kwargs) + ... xblock_info['icon'] = xblock.icon_override + ... return xblock_info """ is_library_block = isinstance(xblock.location, LibraryUsageLocator) is_xblock_unit = is_unit(xblock, parent_xblock) diff --git a/cms/envs/common.py b/cms/envs/common.py index edba66f8b200..79e4906dd6ab 100644 --- a/cms/envs/common.py +++ b/cms/envs/common.py @@ -841,6 +841,7 @@ EditInfoMixin, AuthoringMixin, ) +XBLOCK_EXTRA_MIXINS = () XBLOCK_SELECT_FUNCTION = prefer_xmodules diff --git a/cms/envs/production.py b/cms/envs/production.py index 3c4acdf0f77b..d44c9c59f84b 100644 --- a/cms/envs/production.py +++ b/cms/envs/production.py @@ -598,6 +598,9 @@ def get_env_setting(setting): LOGO_IMAGE_EXTRA_TEXT = ENV_TOKENS.get('LOGO_IMAGE_EXTRA_TEXT', '') +############## XBlock extra mixins ############################ +XBLOCK_MIXINS += tuple(XBLOCK_EXTRA_MIXINS) + ############## Settings for course import olx validation ############################ COURSE_OLX_VALIDATION_STAGE = ENV_TOKENS.get('COURSE_OLX_VALIDATION_STAGE', COURSE_OLX_VALIDATION_STAGE) COURSE_OLX_VALIDATION_IGNORE_LIST = ENV_TOKENS.get( diff --git a/cms/static/js/views/modals/course_outline_modals.js b/cms/static/js/views/modals/course_outline_modals.js index db46623b85aa..43defc2c92cb 100644 --- a/cms/static/js/views/modals/course_outline_modals.js +++ b/cms/static/js/views/modals/course_outline_modals.js @@ -1208,6 +1208,19 @@ define(['jquery', 'backbone', 'underscore', 'gettext', 'js/views/baseview', }, options)); }, + /** + * This function allows comprehensive themes to create custom editors without adding boilerplate code. + * + * A simple example theme for this can be found at https://github.com/open-craft/custom-unit-icons-theme + **/ + getCustomEditModal: function(tabs, editors, xblockInfo, options) { + return new SettingsXBlockModal($.extend({ + tabs: tabs, + editors: editors, + model: xblockInfo + }, options)); + }, + getPublishModal: function(xblockInfo, options) { return new PublishXBlockModal($.extend({ editors: [PublishEditor], diff --git a/cms/static/js/views/pages/course_outline.js b/cms/static/js/views/pages/course_outline.js index cd86d6399795..08f25b3eed19 100644 --- a/cms/static/js/views/pages/course_outline.js +++ b/cms/static/js/views/pages/course_outline.js @@ -34,6 +34,9 @@ define([ collapsedClass: 'is-collapsed' }, + // Extracting this to a variable allows comprehensive themes to replace or extend `CourseOutlineView`. + outlineViewClass: CourseOutlineView, + initialize: function() { var self = this; this.initialState = this.options.initialState; @@ -90,7 +93,7 @@ define([ this.highlightsEnableView.render(); } - this.outlineView = new CourseOutlineView({ + this.outlineView = new this.outlineViewClass({ el: this.$('.outline'), model: this.model, isRoot: true, diff --git a/openedx/core/lib/xblock_utils/__init__.py b/openedx/core/lib/xblock_utils/__init__.py index 1a320f050440..1e2acc7ef84b 100644 --- a/openedx/core/lib/xblock_utils/__init__.py +++ b/openedx/core/lib/xblock_utils/__init__.py @@ -558,6 +558,6 @@ def get_icon(block): """ A function that returns the CSS class representing an icon to use for this particular XBlock (in the courseware navigation bar). Mostly used for Vertical/Unit XBlocks. - It can be overridden by setting `GET_UNIT_ICON_IMPL` to an alternative implementation. + It can be overridden by setting `OVERRIDE_GET_UNIT_ICON` to an alternative implementation. """ return block.get_icon_class() From 96bfcf5848bfea553e43a1801bbd0fba771bd25a Mon Sep 17 00:00:00 2001 From: Agrendalath Date: Sun, 25 Jul 2021 02:06:51 +0200 Subject: [PATCH 11/20] feat: allow marking Library Content Block as complete on view edx/edx-platform#24365 has changed the completion mode of these blocks. Before Koa, it was sufficient to view the block to get a completion checkmark. Since Koa, all children of the block must be completed. This adds a toggle to change the completion behavior back to the previous one so that the user experience can be consistent if needed. (cherry picked from commit d05e5c639f1c58029a75de041a19d0ca47c9f501) --- cms/envs/common.py | 13 +++++++++++++ .../xmodule/xmodule/library_content_module.py | 15 ++++++++++++++- lms/envs/common.py | 13 +++++++++++++ .../completion_integration/test_services.py | 18 ++++++++++++++++++ 4 files changed, 58 insertions(+), 1 deletion(-) diff --git a/cms/envs/common.py b/cms/envs/common.py index 79e4906dd6ab..d69043abf966 100644 --- a/cms/envs/common.py +++ b/cms/envs/common.py @@ -484,6 +484,19 @@ # in the LMS and CMS. # .. toggle_tickets: 'https://github.com/open-craft/edx-platform/pull/429' 'DISABLE_UNENROLLMENT': False, + + # .. toggle_name: MARK_LIBRARY_CONTENT_BLOCK_COMPLETE_ON_VIEW + # .. toggle_implementation: DjangoSetting + # .. toggle_default: False + # .. toggle_description: If enabled, the Library Content Block is marked as complete when users view it. + # Otherwise (by default), all children of this block must be completed. + # .. toggle_use_cases: open_edx + # .. toggle_creation_date: 2022-03-22 + # .. toggle_target_removal_date: None + # .. toggle_tickets: https://github.com/edx/edx-platform/pull/28268 + # .. toggle_warnings: For consistency in user-experience, keep the value in sync with the setting of the same name + # in the LMS and CMS. + 'MARK_LIBRARY_CONTENT_BLOCK_COMPLETE_ON_VIEW': False, } ENABLE_JASMINE = False diff --git a/common/lib/xmodule/xmodule/library_content_module.py b/common/lib/xmodule/xmodule/library_content_module.py index 18dbb9ae11b6..82a23c2b4781 100644 --- a/common/lib/xmodule/xmodule/library_content_module.py +++ b/common/lib/xmodule/xmodule/library_content_module.py @@ -10,6 +10,8 @@ from gettext import ngettext import bleach +from django.conf import settings +from django.utils.functional import classproperty from lazy import lazy from lxml import etree from lxml.etree import XMLSyntaxError @@ -115,7 +117,18 @@ class LibraryContentBlock( show_in_read_only_mode = True - completion_mode = XBlockCompletionMode.AGGREGATOR + # noinspection PyMethodParameters + @classproperty + def completion_mode(cls): # pylint: disable=no-self-argument + """ + Allow overriding the completion mode with a feature flag. + + This is a property, so it can be dynamically overridden in tests, as it is not evaluated at runtime. + """ + if settings.FEATURES.get('MARK_LIBRARY_CONTENT_BLOCK_COMPLETE_ON_VIEW', False): + return XBlockCompletionMode.COMPLETABLE + + return XBlockCompletionMode.AGGREGATOR display_name = String( display_name=_("Display Name"), diff --git a/lms/envs/common.py b/lms/envs/common.py index 18a31ad0e3ad..0b8fde96b271 100644 --- a/lms/envs/common.py +++ b/lms/envs/common.py @@ -960,6 +960,19 @@ # in the LMS and CMS. # .. toggle_tickets: 'https://github.com/open-craft/edx-platform/pull/429' 'DISABLE_UNENROLLMENT': False, + + # .. toggle_name: MARK_LIBRARY_CONTENT_BLOCK_COMPLETE_ON_VIEW + # .. toggle_implementation: DjangoSetting + # .. toggle_default: False + # .. toggle_description: If enabled, the Library Content Block is marked as complete when users view it. + # Otherwise (by default), all children of this block must be completed. + # .. toggle_use_cases: open_edx + # .. toggle_creation_date: 2022-03-22 + # .. toggle_target_removal_date: None + # .. toggle_tickets: https://github.com/edx/edx-platform/pull/28268 + # .. toggle_warnings: For consistency in user-experience, keep the value in sync with the setting of the same name + # in the LMS and CMS. + 'MARK_LIBRARY_CONTENT_BLOCK_COMPLETE_ON_VIEW': False, } # Specifies extra XBlock fields that should available when requested via the Course Blocks API diff --git a/openedx/tests/completion_integration/test_services.py b/openedx/tests/completion_integration/test_services.py index 94fdf4e46b3d..867b89772cd1 100644 --- a/openedx/tests/completion_integration/test_services.py +++ b/openedx/tests/completion_integration/test_services.py @@ -7,6 +7,8 @@ from completion.models import BlockCompletion from completion.services import CompletionService from completion.test_utils import CompletionWaffleTestMixin +from django.conf import settings +from django.test import override_settings from opaque_keys.edx.keys import CourseKey from openedx.core.djangolib.testing.utils import skip_unless_lms @@ -183,6 +185,19 @@ def test_can_mark_block_complete_on_view(self): assert self.completion_service.can_mark_block_complete_on_view(self.html) is True assert self.completion_service.can_mark_block_complete_on_view(self.problem) is False + @override_settings(FEATURES={**settings.FEATURES, 'MARK_LIBRARY_CONTENT_BLOCK_COMPLETE_ON_VIEW': True}) + def test_can_mark_library_content_complete_on_view(self): + library = LibraryFactory.create(modulestore=self.store) + lib_vertical = ItemFactory.create(parent=self.sequence, category='vertical', publish_item=False) + library_content_block = ItemFactory.create( + parent=lib_vertical, + category='library_content', + max_count=1, + source_library_id=str(library.location.library_key), + user_id=self.user.id, + ) + self.assertTrue(self.completion_service.can_mark_block_complete_on_view(library_content_block)) + def test_vertical_completion_with_library_content(self): library = LibraryFactory.create(modulestore=self.store) ItemFactory.create(parent=library, category='problem', publish_item=False, user_id=self.user.id) @@ -204,6 +219,9 @@ def test_vertical_completion_with_library_content(self): source_library_id=str(library.location.library_key), user_id=self.user.id, ) + # Library Content Block needs its children to be completed. + self.assertFalse(self.completion_service.can_mark_block_complete_on_view(library_content_block)) + library_content_block.refresh_children() lib_vertical = self.store.get_item(lib_vertical.location) self._bind_course_module(lib_vertical) From 71c8693eaf762db782e1d06f3945a4b133e68a61 Mon Sep 17 00:00:00 2001 From: Agrendalath Date: Mon, 25 Oct 2021 21:15:45 +0200 Subject: [PATCH 12/20] fix: do not index solutions in CAPA blocks Most tags that could contain solutions or hints were already being removed, but the regex did not include the case when they contained attributes. (cherry picked from commit 0ef57eb136914a4dd0de1396095c80d8eddf73ac) --- common/lib/xmodule/xmodule/capa_module.py | 18 +++++-- .../xmodule/xmodule/tests/test_capa_module.py | 47 +++++++++++-------- 2 files changed, 40 insertions(+), 25 deletions(-) diff --git a/common/lib/xmodule/xmodule/capa_module.py b/common/lib/xmodule/xmodule/capa_module.py index 3311e2eb85f8..43503de0de82 100644 --- a/common/lib/xmodule/xmodule/capa_module.py +++ b/common/lib/xmodule/xmodule/capa_module.py @@ -548,14 +548,22 @@ def index_dictionary(self): # Make optioninput's options index friendly by replacing the actual tag with the values capa_content = re.sub(r'\s*|\S*<\/optioninput>', r'\1', self.data) - # Removing solutions and hints, as well as script and style + # Remove the following tags with content that can leak hints or solutions: + # - `solution` (with optional attributes) and `solutionset`. + # - `targetedfeedback` (with optional attributes) and `targetedfeedbackset`. + # - `answer` (with optional attributes). + # - `script` (with optional attributes). + # - `style` (with optional attributes). + # - various types of hints (with optional attributes) and `hintpart`. capa_content = re.sub( re.compile( r""" - .*? | - | - | - <[a-z]*hint.*?>.*? + .*? | + .*? | + .*? | + .*? | + .*? | + <[a-z]*hint.*?>.*? """, re.DOTALL | re.VERBOSE), diff --git a/common/lib/xmodule/xmodule/tests/test_capa_module.py b/common/lib/xmodule/xmodule/tests/test_capa_module.py index 30f2e5cd4bc5..4667dc4f92e8 100644 --- a/common/lib/xmodule/xmodule/tests/test_capa_module.py +++ b/common/lib/xmodule/xmodule/tests/test_capa_module.py @@ -2561,25 +2561,32 @@ def test_response_types_multiple_tags(self): def test_solutions_not_indexed(self): xml = textwrap.dedent(""" - -
-

Explanation

- -

This is what the 1st solution.

- -
- - - -
-

Explanation

- -

This is the 2nd solution.

- -
- - - + Test solution. + Test solution with attribute. + + Test solutionset. + Test solution within solutionset. + + + Test feedback. + Test feedback with attribute. + + Test FeedbackSet. + Test feedback within feedbackset. + + + Test answer. + Test answer with attribute. + + + + + + + + Test choicehint. + Test hint. + Test hintpart. """) name = "Blank Common Capa Problem" @@ -2694,7 +2701,7 @@ def test_indexing_non_latin_problem(self): """) name = "Non latin Input" descriptor = self._create_descriptor(sample_text_input_problem_xml, name=name) - capa_content = " FX1_VAL='Καλημέρα' Δοκιμή με μεταβλητές με Ελληνικούς χαρακτήρες μέσα σε python: $FX1_VAL " + capa_content = " Δοκιμή με μεταβλητές με Ελληνικούς χαρακτήρες μέσα σε python: $FX1_VAL " descriptor_dict = descriptor.index_dictionary() assert descriptor_dict['content']['capa_content'] == smart_str(capa_content) From 87424a155b53a47f017523062b8abfbfb75e242e Mon Sep 17 00:00:00 2001 From: Arunmozhi Date: Wed, 1 Sep 2021 13:47:39 +0000 Subject: [PATCH 13/20] feat: add reset option to Randomized Content Block This makes the reset button to refresh the contents of a Randomized Content Block (RCB) without reloading the full page by fetching a new set of problems in the "reset" response and replacing the DOM contents. The reset button returns the student view as a string and the client uses the HtmlUtils package to replace the contents and reinitializes the XBlock. This allows students to use the RCB as a flash card system. Co-authored-by: tinumide --- .../public/js/library_content_reset.js | 18 +++++++ .../xmodule/xmodule/library_content_module.py | 36 +++++++++++++- .../xmodule/tests/test_library_content.py | 47 ++++++++++++++++++- .../sass/course/courseware/_courseware.scss | 11 +++++ lms/templates/vert_module.html | 6 +++ 5 files changed, 116 insertions(+), 2 deletions(-) create mode 100644 common/lib/xmodule/xmodule/assets/library_content/public/js/library_content_reset.js diff --git a/common/lib/xmodule/xmodule/assets/library_content/public/js/library_content_reset.js b/common/lib/xmodule/xmodule/assets/library_content/public/js/library_content_reset.js new file mode 100644 index 000000000000..e985d3c2a692 --- /dev/null +++ b/common/lib/xmodule/xmodule/assets/library_content/public/js/library_content_reset.js @@ -0,0 +1,18 @@ +/* JavaScript for reset option that can be done on a randomized LibraryContentBlock */ +function LibraryContentReset(runtime, element) { + $('.problem-reset-btn', element).click((e) => { + e.preventDefault(); + $.post({ + url: runtime.handlerUrl(element, 'reset_selected_children'), + success(data) { + edx.HtmlUtils.setHtml(element, edx.HtmlUtils.HTML(data)); + // Rebind the reset button for the block + XBlock.initializeBlock(element); + // Render the new set of problems (XBlocks) + $(".xblock", element).each(function(i, child) { + XBlock.initializeBlock(child); + }); + }, + }); + }); +} diff --git a/common/lib/xmodule/xmodule/library_content_module.py b/common/lib/xmodule/xmodule/library_content_module.py index 82a23c2b4781..c1ba0d1807fc 100644 --- a/common/lib/xmodule/xmodule/library_content_module.py +++ b/common/lib/xmodule/xmodule/library_content_module.py @@ -8,6 +8,7 @@ import random from copy import copy from gettext import ngettext +from rest_framework import status import bleach from django.conf import settings @@ -21,7 +22,7 @@ from webob import Response from xblock.completable import XBlockCompletionMode from xblock.core import XBlock -from xblock.fields import Integer, List, Scope, String +from xblock.fields import Integer, List, Scope, String, Boolean from capa.responsetypes import registry from xmodule.mako_module import MakoTemplateBlockBase @@ -177,6 +178,14 @@ def completion_mode(cls): # pylint: disable=no-self-argument default=[], scope=Scope.user_state, ) + # This cannot be called `show_reset_button`, because children blocks inherit this as a default value. + allow_resetting_children = Boolean( + display_name=_("Show Reset Button"), + help=_("Determines whether a 'Reset Problems' button is shown, so users may reset their answers and reshuffle " + "selected items."), + scope=Scope.settings, + default=False + ) @property def source_library_key(self): @@ -347,6 +356,27 @@ def selected_children(self): return self.selected + @XBlock.handler + def reset_selected_children(self, _, __): + """ + Resets the XBlock's state for a user. + + This resets the state of all `selected` children and then clears the `selected` field + so that the new blocks are randomly chosen for this user. + """ + if not self.allow_resetting_children: + return Response('"Resetting selected children" is not allowed for this XBlock', + status=status.HTTP_400_BAD_REQUEST) + + for block_type, block_id in self.selected_children(): + block = self.runtime.get_block(self.location.course_key.make_usage_key(block_type, block_id)) + if hasattr(block, 'reset_problem'): + block.reset_problem(None) + block.save() + + self.selected = [] + return Response(json.dumps(self.student_view({}).content)) + def _get_selected_child_blocks(self): """ Generator returning XBlock instances of the children selected for the @@ -384,7 +414,11 @@ def student_view(self, context): # lint-amnesty, pylint: disable=missing-functi 'show_bookmark_button': False, 'watched_completable_blocks': set(), 'completion_delay_ms': None, + 'reset_button': self.allow_resetting_children, })) + + fragment.add_javascript_url(self.runtime.local_resource_url(self, 'public/js/library_content_reset.js')) + fragment.initialize_js('LibraryContentReset') return fragment def author_view(self, context): diff --git a/common/lib/xmodule/xmodule/tests/test_library_content.py b/common/lib/xmodule/xmodule/tests/test_library_content.py index 628f471e88eb..748a962f1d65 100644 --- a/common/lib/xmodule/xmodule/tests/test_library_content.py +++ b/common/lib/xmodule/xmodule/tests/test_library_content.py @@ -3,7 +3,8 @@ Higher-level tests are in `cms/djangoapps/contentstore/tests/test_libraries.py`. """ -from unittest.mock import Mock, patch +import ddt +from unittest.mock import MagicMock, Mock, patch from bson.objectid import ObjectId from fs.memoryfs import MemoryFS @@ -11,6 +12,7 @@ from search.search_engine_base import SearchEngine from web_fragments.fragment import Fragment from xblock.runtime import Runtime as VanillaRuntime +from rest_framework import status from xmodule.library_content_module import ANY_CAPA_TYPE_VALUE, LibraryContentBlock from xmodule.library_tools import LibraryToolsService @@ -20,6 +22,7 @@ from xmodule.tests import get_test_system from xmodule.validation import StudioValidationMessage from xmodule.x_module import AUTHOR_VIEW +from xmodule.capa_module import ProblemBlock from .test_course_module import DummySystem as TestImportSystem @@ -30,6 +33,7 @@ class LibraryContentTest(MixedSplitTestCase): """ Base class for tests of LibraryContentBlock (library_content_block.py) """ + def setUp(self): super().setUp() @@ -164,6 +168,7 @@ def test_xml_import_with_comments(self): self._verify_xblock_properties(imported_lc_block) +@ddt.ddt class LibraryContentBlockTestMixin: """ Basic unit tests for LibraryContentBlock @@ -378,6 +383,45 @@ def _change_count_and_refresh_children(self, count): assert len(selected) == count return selected + @ddt.data( + # User resets selected children with reset button on content block + (True, 8), + # User resets selected children without reset button on content block + (False, 8), + ) + @ddt.unpack + def test_reset_selected_children_capa_blocks(self, allow_resetting_children, max_count): + """ + Tests that the `reset_selected_children` method of a content block resets only + XBlocks that have a `reset_problem` attribute when `allow_resetting_children` is True + + This test block has 4 HTML XBlocks and 4 Problem XBlocks. Therefore, if we ensure + that the `reset_problem` has been called len(self.problem_types) times, then + it means that this is working correctly + """ + self.lc_block.allow_resetting_children = allow_resetting_children + self.lc_block.max_count = max_count + # Add some capa blocks + self._create_capa_problems() + self.lc_block.refresh_children() + self.lc_block = self.store.get_item(self.lc_block.location) + # Mock the student view to return an empty dict to be returned as response + self.lc_block.student_view = MagicMock() + self.lc_block.student_view.return_value.content = {} + + with patch.object(ProblemBlock, 'reset_problem', return_value={'success': True}) as reset_problem: + response = self.lc_block.reset_selected_children(None, None) + + if allow_resetting_children: + self.lc_block.student_view.assert_called_once_with({}) + assert reset_problem.call_count == len(self.problem_types) + assert response.status_code == status.HTTP_200_OK + assert response.content_type == "text/html" + assert response.body == b"{}" + else: + reset_problem.assert_not_called() + assert response.status_code == status.HTTP_400_BAD_REQUEST + @patch('xmodule.library_tools.SearchEngine.get_search_engine', Mock(return_value=None, autospec=True)) class TestLibraryContentBlockNoSearchIndex(LibraryContentBlockTestMixin, LibraryContentTest): @@ -396,6 +440,7 @@ class TestLibraryContentBlockWithSearchIndex(LibraryContentBlockTestMixin, Libra """ Tests for library container with mocked search engine response. """ + def _get_search_response(self, field_dictionary=None): """ Mocks search response as returned by search engine """ target_type = field_dictionary.get('problem_types') diff --git a/lms/static/sass/course/courseware/_courseware.scss b/lms/static/sass/course/courseware/_courseware.scss index 67a5a704050b..33f8dae798ad 100644 --- a/lms/static/sass/course/courseware/_courseware.scss +++ b/lms/static/sass/course/courseware/_courseware.scss @@ -635,6 +635,17 @@ html.video-fullscreen { border-bottom: 1px solid #ddd; margin-bottom: ($baseline*0.75); padding: 0 0 15px; + + .problem-reset-btn-wrapper { + position: relative; + .problem-reset-btn { + &:hover, + &:focus, + &:active { + color: $primary; + } + } + } } .vert > .xblock-student_view.is-hidden, diff --git a/lms/templates/vert_module.html b/lms/templates/vert_module.html index 131bbfc8cadc..0e52e3c7f426 100644 --- a/lms/templates/vert_module.html +++ b/lms/templates/vert_module.html @@ -69,6 +69,12 @@

${unit_title}

% endfor +% if reset_button: +
+ +
+% endif + <%static:require_module_async module_name="js/dateutil_factory" class_name="DateUtilFactory"> DateUtilFactory.transform('.localized-datetime'); From fd8257a99e9c6e56dd5bd77628620ea7255c280a Mon Sep 17 00:00:00 2001 From: Sandeep Kumar Choudhary Date: Mon, 7 Jun 2021 05:52:39 +0000 Subject: [PATCH 14/20] feat: Allow delete course content in Studio only for admin users (cherry picked from commit c812a6c1d5c0961900507a6e7abe3d0f3b8a7570) --- cms/djangoapps/contentstore/config/waffle.py | 16 +- cms/djangoapps/contentstore/permissions.py | 10 + cms/djangoapps/contentstore/views/item.py | 9 +- .../contentstore/views/tests/test_item.py | 171 ++++++++++++++++-- .../spec/views/pages/course_outline_spec.js | 22 ++- cms/static/js/views/xblock_outline.js | 3 +- cms/templates/js/course-outline.underscore | 2 +- 7 files changed, 209 insertions(+), 24 deletions(-) create mode 100644 cms/djangoapps/contentstore/permissions.py diff --git a/cms/djangoapps/contentstore/config/waffle.py b/cms/djangoapps/contentstore/config/waffle.py index 3dc567a14f0e..6a8c33aa98be 100644 --- a/cms/djangoapps/contentstore/config/waffle.py +++ b/cms/djangoapps/contentstore/config/waffle.py @@ -19,11 +19,9 @@ def waffle(): """ Deprecated: Returns the namespaced, cached, audited Waffle Switch class for Studio pages. - IMPORTANT: Do NOT copy this pattern and do NOT use this to reference new switches. Instead, replace the string constant above with the actual switch instance. For example:: - ENABLE_ACCESSIBILITY_POLICY_PAGE = WaffleSwitch(f'{WAFFLE_NAMESPACE}.enable_policy_page') """ return LegacyWaffleSwitchNamespace(name=WAFFLE_NAMESPACE, log_prefix='Studio: ') @@ -32,13 +30,11 @@ def waffle(): def waffle_flags(): """ Deprecated: Returns the namespaced, cached, audited Waffle Flag class for Studio pages. - IMPORTANT: Do NOT copy this pattern and do NOT use this to reference new flags. See waffle() docstring for more details. """ return LegacyWaffleFlagNamespace(name=WAFFLE_NAMESPACE, log_prefix='Studio: ') - # TODO: After removing this flag, add a migration to remove waffle flag in a follow-up deployment. ENABLE_CHECKLISTS_QUALITY = CourseWaffleFlag( # lint-amnesty, pylint: disable=toggle-missing-annotation waffle_namespace=waffle_flags(), @@ -81,3 +77,15 @@ def waffle_flags(): # .. toggle_warnings: Flag course_experience.relative_dates should also be active for relative dates functionalities to work. # .. toggle_tickets: https://openedx.atlassian.net/browse/AA-844 CUSTOM_RELATIVE_DATES = CourseWaffleFlag(WAFFLE_NAMESPACE, 'custom_relative_dates', module_name=__name__,) + +# .. toggle_name: studio.prevent_staff_structure_deletion +# .. toggle_implementation: WaffleFlag +# .. toggle_default: False +# .. toggle_description: Prevents staff from deleting course structures +# .. toggle_use_cases: opt_in +# .. toggle_creation_date: 2021-06-25 +PREVENT_STAFF_STRUCTURE_DELETION = LegacyWaffleFlag( + waffle_namespace=waffle_flags(), + flag_name='prevent_staff_structure_deletion', + module_name=__name__, +) diff --git a/cms/djangoapps/contentstore/permissions.py b/cms/djangoapps/contentstore/permissions.py new file mode 100644 index 000000000000..14fe40c09ca7 --- /dev/null +++ b/cms/djangoapps/contentstore/permissions.py @@ -0,0 +1,10 @@ +""" +Permission definitions for the contentstore djangoapp +""" + +from bridgekeeper import perms + +from lms.djangoapps.courseware.rules import HasRolesRule + +DELETE_COURSE_CONTENT = 'contentstore.delete_course_content' +perms[DELETE_COURSE_CONTENT] = HasRolesRule('instructor') diff --git a/cms/djangoapps/contentstore/views/item.py b/cms/djangoapps/contentstore/views/item.py index 0d526bef08c8..0c02bdbefb58 100644 --- a/cms/djangoapps/contentstore/views/item.py +++ b/cms/djangoapps/contentstore/views/item.py @@ -29,7 +29,8 @@ from xblock.core import XBlock from xblock.fields import Scope -from cms.djangoapps.contentstore.config.waffle import SHOW_REVIEW_RULES_FLAG +from cms.djangoapps.contentstore.config.waffle import PREVENT_STAFF_STRUCTURE_DELETION, SHOW_REVIEW_RULES_FLAG +from cms.djangoapps.contentstore.permissions import DELETE_COURSE_CONTENT from cms.djangoapps.models.settings.course_grading import CourseGradingModel from cms.lib.xblock.authoring_mixin import VISIBILITY_VIEW from common.djangoapps.edxmako.shortcuts import render_to_string @@ -1339,6 +1340,12 @@ def create_xblock_info(xblock, data=None, metadata=None, include_ancestor_info=F else: xblock_info['staff_only_message'] = False + xblock_info['show_delete_button'] = True + if PREVENT_STAFF_STRUCTURE_DELETION.is_enabled(): + xblock_info['show_delete_button'] = ( + user.has_perm(DELETE_COURSE_CONTENT, xblock) if user is not None else False + ) + xblock_info['has_partition_group_components'] = has_children_visible_to_specific_partition_groups( xblock ) diff --git a/cms/djangoapps/contentstore/views/tests/test_item.py b/cms/djangoapps/contentstore/views/tests/test_item.py index 0d93d8b4545d..c1cf8e9cedf7 100644 --- a/cms/djangoapps/contentstore/views/tests/test_item.py +++ b/cms/djangoapps/contentstore/views/tests/test_item.py @@ -13,6 +13,7 @@ from django.test.client import RequestFactory from django.urls import reverse from edx_proctoring.exceptions import ProctoredExamNotFoundException +from edx_toggles.toggles.testutils import override_waffle_flag from opaque_keys import InvalidKeyError from opaque_keys.edx.asides import AsideUsageKeyV2 from opaque_keys.edx.keys import CourseKey, UsageKey @@ -27,18 +28,6 @@ from xblock.runtime import DictKeyValueStore, KvsFieldData from xblock.test.tools import TestRuntime from xblock.validation import ValidationMessage - -from cms.djangoapps.contentstore.tests.utils import CourseTestCase -from cms.djangoapps.contentstore.utils import reverse_course_url, reverse_usage_url -from cms.djangoapps.contentstore.views import item as item_module -from common.djangoapps.student.tests.factories import UserFactory -from common.djangoapps.xblock_django.models import ( - XBlockConfiguration, - XBlockStudioConfiguration, - XBlockStudioConfigurationFlag -) -from common.djangoapps.xblock_django.user_service import DjangoXBlockUserService -from lms.djangoapps.lms_xblock.mixin import NONSENSICAL_ACCESS_RESTRICTION from xmodule.capa_module import ProblemBlock from xmodule.course_module import DEFAULT_START_DATE from xmodule.modulestore import ModuleStoreEnum @@ -55,6 +44,20 @@ from xmodule.partitions.tests.test_partitions import MockPartitionService from xmodule.x_module import STUDENT_VIEW, STUDIO_VIEW +from cms.djangoapps.contentstore.tests.utils import CourseTestCase +from cms.djangoapps.contentstore.utils import reverse_course_url, reverse_usage_url +from cms.djangoapps.contentstore.views import item as item_module +from cms.djangoapps.contentstore.config.waffle import PREVENT_STAFF_STRUCTURE_DELETION +from common.djangoapps.student.roles import CourseInstructorRole, CourseStaffRole, CourseCreatorRole +from common.djangoapps.student.tests.factories import UserFactory +from common.djangoapps.xblock_django.models import ( + XBlockConfiguration, + XBlockStudioConfiguration, + XBlockStudioConfigurationFlag +) +from common.djangoapps.xblock_django.user_service import DjangoXBlockUserService +from lms.djangoapps.lms_xblock.mixin import NONSENSICAL_ACCESS_RESTRICTION + from ..component import component_handler, get_component_templates from ..item import ( ALWAYS, @@ -3390,3 +3393,147 @@ def test_self_paced_item_visibility_state(self, store_type): # Check that in self paced course content has live state now xblock_info = self._get_xblock_info(chapter.location) self._verify_visibility_state(xblock_info, VisibilityState.live) + + def test_staff_show_delete_button(self): + """ + Test delete button is *not visible* to user with CourseStaffRole + """ + # Add user as course staff + CourseStaffRole(self.course_key).add_users(self.user) + + # Get xblock outline + xblock_info = create_xblock_info( + self.course, + include_child_info=True, + course_outline=True, + include_children_predicate=lambda xblock: not xblock.category == 'vertical', + user=self.user + ) + self.assertTrue(xblock_info['show_delete_button']) + + def test_staff_show_delete_button_with_waffle(self): + """ + Test delete button is *not visible* to user with CourseStaffRole and + PREVENT_STAFF_STRUCTURE_DELETION waffle set + """ + # Add user as course staff + CourseStaffRole(self.course_key).add_users(self.user) + + with override_waffle_flag(PREVENT_STAFF_STRUCTURE_DELETION, active=True): + # Get xblock outline + xblock_info = create_xblock_info( + self.course, + include_child_info=True, + course_outline=True, + include_children_predicate=lambda xblock: not xblock.category == 'vertical', + user=self.user + ) + + self.assertFalse(xblock_info['show_delete_button']) + + def test_no_user_show_delete_button(self): + """ + Test delete button is *visible* when user attribute is not set on + xblock. This happens with ajax requests. + """ + # Get xblock outline + xblock_info = create_xblock_info( + self.course, + include_child_info=True, + course_outline=True, + include_children_predicate=lambda xblock: not xblock.category == 'vertical', + user=None + ) + self.assertTrue(xblock_info['show_delete_button']) + + def test_no_user_show_delete_button_with_waffle(self): + """ + Test delete button is *visible* when user attribute is not set on + xblock (this happens with ajax requests) and PREVENT_STAFF_STRUCTURE_DELETION waffle set. + """ + + with override_waffle_flag(PREVENT_STAFF_STRUCTURE_DELETION, active=True): + # Get xblock outline + xblock_info = create_xblock_info( + self.course, + include_child_info=True, + course_outline=True, + include_children_predicate=lambda xblock: not xblock.category == 'vertical', + user=None + ) + + self.assertFalse(xblock_info['show_delete_button']) + + def test_instructor_show_delete_button(self): + """ + Test delete button is *visible* to user with CourseInstructorRole only + """ + # Add user as course instructor + CourseInstructorRole(self.course_key).add_users(self.user) + + # Get xblock outline + xblock_info = create_xblock_info( + self.course, + include_child_info=True, + course_outline=True, + include_children_predicate=lambda xblock: not xblock.category == 'vertical', + user=self.user + ) + self.assertTrue(xblock_info['show_delete_button']) + + def test_instructor_show_delete_button_with_waffle(self): + """ + Test delete button is *visible* to user with CourseInstructorRole only + and PREVENT_STAFF_STRUCTURE_DELETION waffle set + """ + # Add user as course instructor + CourseInstructorRole(self.course_key).add_users(self.user) + + with override_waffle_flag(PREVENT_STAFF_STRUCTURE_DELETION, active=True): + # Get xblock outline + xblock_info = create_xblock_info( + self.course, + include_child_info=True, + course_outline=True, + include_children_predicate=lambda xblock: not xblock.category == 'vertical', + user=self.user + ) + + self.assertTrue(xblock_info['show_delete_button']) + + def test_creator_show_delete_button(self): + """ + Test delete button is *visible* to user with CourseInstructorRole only + """ + # Add user as course creator + CourseCreatorRole(self.course_key).add_users(self.user) + + # Get xblock outline + xblock_info = create_xblock_info( + self.course, + include_child_info=True, + course_outline=True, + include_children_predicate=lambda xblock: not xblock.category == 'vertical', + user=self.user + ) + self.assertTrue(xblock_info['show_delete_button']) + + def test_creator_show_delete_button_with_waffle(self): + """ + Test delete button is *visible* to user with CourseInstructorRole only + and PREVENT_STAFF_STRUCTURE_DELETION waffle set + """ + # Add user as course creator + CourseCreatorRole(self.course_key).add_users(self.user) + + with override_waffle_flag(PREVENT_STAFF_STRUCTURE_DELETION, active=True): + # Get xblock outline + xblock_info = create_xblock_info( + self.course, + include_child_info=True, + course_outline=True, + include_children_predicate=lambda xblock: not xblock.category == 'vertical', + user=self.user + ) + + self.assertFalse(xblock_info['show_delete_button']) diff --git a/cms/static/js/spec/views/pages/course_outline_spec.js b/cms/static/js/spec/views/pages/course_outline_spec.js index b4fd8143b768..f6388d6f30cb 100644 --- a/cms/static/js/spec/views/pages/course_outline_spec.js +++ b/cms/static/js/spec/views/pages/course_outline_spec.js @@ -40,7 +40,8 @@ describe('CourseOutlinePage', function() { user_partitions: [], user_partition_info: {}, highlights_enabled: true, - highlights_enabled_for_messaging: false + highlights_enabled_for_messaging: false, + show_delete_button: true }, options, {child_info: {children: children}}); }; @@ -67,7 +68,8 @@ describe('CourseOutlinePage', function() { show_review_rules: true, user_partition_info: {}, highlights_enabled: true, - highlights_enabled_for_messaging: false + highlights_enabled_for_messaging: false, + show_delete_button: true }, options, {child_info: {children: children}}); }; @@ -92,7 +94,8 @@ describe('CourseOutlinePage', function() { group_access: {}, user_partition_info: {}, highlights: [], - highlights_enabled: true + highlights_enabled: true, + show_delete_button: true }, options, {child_info: {children: children}}); }; @@ -122,7 +125,8 @@ describe('CourseOutlinePage', function() { }, user_partitions: [], group_access: {}, - user_partition_info: {} + user_partition_info: {}, + show_delete_button: true }, options, {child_info: {children: children}}); }; @@ -140,7 +144,8 @@ describe('CourseOutlinePage', function() { edited_by: 'MockUser', user_partitions: [], group_access: {}, - user_partition_info: {} + user_partition_info: {}, + show_delete_button: true }, options); }; @@ -857,6 +862,13 @@ describe('CourseOutlinePage', function() { expect(outlinePage.$('[data-locator="mock-section-2"]')).toExist(); }); + it('remains un-visible if show_delete_button is false ', function() { + createCourseOutlinePage(this, createMockCourseJSON({show_delete_button: false}, [ + createMockSectionJSON({show_delete_button: false}) + ])); + expect(getItemHeaders('section').find('.delete-button').first()).not.toExist(); + }); + it('can be deleted if it is the only section', function() { var promptSpy = EditHelpers.createPromptSpy(); createCourseOutlinePage(this, mockSingleSectionCourseJSON); diff --git a/cms/static/js/views/xblock_outline.js b/cms/static/js/views/xblock_outline.js index badf43dc1fa9..2d63ec774909 100644 --- a/cms/static/js/views/xblock_outline.js +++ b/cms/static/js/views/xblock_outline.js @@ -109,7 +109,8 @@ define(['jquery', 'underscore', 'gettext', 'js/views/baseview', 'common/js/compo includesChildren: this.shouldRenderChildren(), hasExplicitStaffLock: this.model.get('has_explicit_staff_lock'), staffOnlyMessage: this.model.get('staff_only_message'), - course: course + course: course, + showDeleteButton: this.model.get('show_delete_button') }; }, diff --git a/cms/templates/js/course-outline.underscore b/cms/templates/js/course-outline.underscore index df43d0913bba..23dd9f8efff7 100644 --- a/cms/templates/js/course-outline.underscore +++ b/cms/templates/js/course-outline.underscore @@ -161,7 +161,7 @@ if (is_proctored_exam) { <% } %> - <% if (xblockInfo.isDeletable()) { %> + <% if (xblockInfo.isDeletable() && showDeleteButton) { %>
  • From fd368922ec5b43baa075905dfa4ad18e97d8de1a Mon Sep 17 00:00:00 2001 From: pkulkark Date: Tue, 1 Feb 2022 12:12:56 +0530 Subject: [PATCH 15/20] feat: Make course description editable in certs Adds the ability to edit the default course description shown in certificates. (cherry picked from a89baafe0575c94fac0cb4ce9db1d38ce0b71bc6) --- .../contentstore/views/certificates.py | 2 ++ .../js/certificates/models/certificate.js | 1 + .../certificates/views/certificate_editor.js | 13 ++++++++++ .../js/certificate-details.underscore | 6 +++++ .../js/certificate-editor.underscore | 5 ++++ .../certificates/tests/test_webview_views.py | 18 +++---------- lms/djangoapps/certificates/views/webview.py | 26 ++++++++++++------- 7 files changed, 47 insertions(+), 24 deletions(-) diff --git a/cms/djangoapps/contentstore/views/certificates.py b/cms/djangoapps/contentstore/views/certificates.py index 353e71fe6289..6304bbe23ab4 100644 --- a/cms/djangoapps/contentstore/views/certificates.py +++ b/cms/djangoapps/contentstore/views/certificates.py @@ -231,6 +231,8 @@ def serialize_certificate(certificate): # Some keys are not required, such as the title override... if certificate_data.get('course_title'): certificate_response["course_title"] = certificate_data['course_title'] + if certificate_data.get('course_description'): + certificate_response['course_description'] = certificate_data['course_description'] return certificate_response diff --git a/cms/static/js/certificates/models/certificate.js b/cms/static/js/certificates/models/certificate.js index a440d569d606..cecdd26f071f 100644 --- a/cms/static/js/certificates/models/certificate.js +++ b/cms/static/js/certificates/models/certificate.js @@ -18,6 +18,7 @@ define([ defaults: { // Metadata fields currently displayed in web forms course_title: '', + course_description: '', // Metadata fields not currently displayed in web forms name: 'Name of the certificate', diff --git a/cms/static/js/certificates/views/certificate_editor.js b/cms/static/js/certificates/views/certificate_editor.js index fa19bd2de258..bc2b0f85ed5f 100644 --- a/cms/static/js/certificates/views/certificate_editor.js +++ b/cms/static/js/certificates/views/certificate_editor.js @@ -24,6 +24,7 @@ function($, _, Backbone, gettext, 'change .collection-name-input': 'setName', 'change .certificate-description-input': 'setDescription', 'change .certificate-course-title-input': 'setCourseTitle', + 'change .certificate-course-description-input': 'setCourseDescription', 'focus .input-text': 'onFocus', 'blur .input-text': 'onBlur', submit: 'setAndClose', @@ -103,6 +104,7 @@ function($, _, Backbone, gettext, name: this.model.get('name'), description: this.model.get('description'), course_title: this.model.get('course_title'), + course_description: this.model.get('course_description'), org_logo_path: this.model.get('org_logo_path'), is_active: this.model.get('is_active'), isNew: this.model.isNew() @@ -143,11 +145,22 @@ function($, _, Backbone, gettext, ); }, + setCourseDescription: function(event) { + // Updates the indicated model field (still requires persistence on server) + if (event && event.preventDefault) { event.preventDefault(); } + this.model.set( + 'course_description', + this.$('.certificate-course-description-input').val(), + {silent: true} + ); + }, + setValues: function() { // Update the specified values in the local model instance this.setName(); this.setDescription(); this.setCourseTitle(); + this.setCourseDescription(); return this; } }); diff --git a/cms/templates/js/certificate-details.underscore b/cms/templates/js/certificate-details.underscore index a09a3baf897c..3401fd175a39 100644 --- a/cms/templates/js/certificate-details.underscore +++ b/cms/templates/js/certificate-details.underscore @@ -29,6 +29,12 @@ <%- course_title %>

    <% } %> + <% if (course_description) { %> +

    + <%- gettext('Course Description') %>: + <%- course_description %> +

    + <% } %>
    diff --git a/cms/templates/js/certificate-editor.underscore b/cms/templates/js/certificate-editor.underscore index 513113b80500..3b1d90969b5a 100644 --- a/cms/templates/js/certificate-editor.underscore +++ b/cms/templates/js/certificate-editor.underscore @@ -31,6 +31,11 @@ " value="<%- course_title %>" aria-describedby="certificate-course-title-<%-uniqueId %>-tip" /> <%- gettext("Specify an alternative to the official course title to display on certificates. Leave blank to use the official course title.") %>
    +
    + + " value="<%- course_description %>" aria-describedby="certificate-course-description-<%-uniqueId %>-tip" /> + <%- gettext("Specify an alternative to the official course description to display on certificates. Leave blank to use default text.") %> +

    <%- gettext("Certificate Signatories") %>

    diff --git a/lms/djangoapps/certificates/tests/test_webview_views.py b/lms/djangoapps/certificates/tests/test_webview_views.py index 1a23eb85224a..ff22e8c33df5 100644 --- a/lms/djangoapps/certificates/tests/test_webview_views.py +++ b/lms/djangoapps/certificates/tests/test_webview_views.py @@ -139,6 +139,7 @@ def _add_course_certificates(self, count=1, signatory_count=0, is_active=True): 'name': 'Name ' + str(i), 'description': 'Description ' + str(i), 'course_title': 'course_title_' + str(i), + 'course_description': 'course_description_' + str(i), 'org_logo_path': f'/t4x/orgX/testX/asset/org-logo-{i}.png', 'signatories': signatories, 'version': 1, @@ -426,11 +427,6 @@ def test_rendering_course_organization_data(self): uuid=self.cert.verify_uuid ) response = self.client.get(test_url) - self.assertContains( - response, - 'a course of study offered by test_organization, an online learning initiative of test organization', - ) - self.assertNotContains(response, 'a course of study offered by testorg') self.assertContains(response, f'test_organization {self.course.number} Certificate |') self.assertContains(response, 'logo_test1.png') @@ -515,21 +511,13 @@ def test_rendering_maximum_data(self): self.assertContains(response, '<a class="logo" href="http://test_site.localhost">') # Test an item from course info self.assertContains(response, 'course_title_0') + # Test an item from course description + self.assertContains(response, 'course_description_0') # Test an item from user info self.assertContains(response, f"{self.user.profile.name}, you earned a certificate!") # Test an item from social info self.assertContains(response, "Post on Facebook") self.assertContains(response, "Share on Twitter") - # Test an item from certificate/org info - self.assertContains( - response, - "a course of study offered by {partner_short_name}, " - "an online learning initiative of " - "{partner_long_name}.".format( - partner_short_name=short_org_name, - partner_long_name=long_org_name, - ), - ) # Test item from badge info self.assertContains(response, "Add to Mozilla Backpack") # Test item from site configuration diff --git a/lms/djangoapps/certificates/views/webview.py b/lms/djangoapps/certificates/views/webview.py index 285a65caecd5..25a445a12644 100644 --- a/lms/djangoapps/certificates/views/webview.py +++ b/lms/djangoapps/certificates/views/webview.py @@ -55,7 +55,7 @@ from openedx.core.djangoapps.site_configuration import helpers as configuration_helpers from openedx.core.lib.courses import course_image_url from openedx.core.lib.courses import get_course_by_id -from xmodule.data import CertificatesDisplayBehaviors +from xmodule.data import CertificatesDisplayBehaviors # lint-amnesty, pylint: disable=wrong-import-order log = logging.getLogger(__name__) _ = translation.gettext @@ -74,18 +74,23 @@ def get_certificate_description(mode, certificate_type, platform_name): certificate_type_description = _("An {cert_type} certificate signifies that a " "learner has agreed to abide by the honor code established by " "{platform_name} and has completed all of the required tasks for this course " - "under its guidelines.").format(cert_type=certificate_type, - platform_name=platform_name) + "under its guidelines. A {cert_type} certificate also indicates that the " + "identity of the learner has been checked and " + "is valid.").format(cert_type=certificate_type, + platform_name=platform_name) elif mode == 'verified': # Translators: This text describes the 'ID Verified' course certificate type, which is a higher level of # verification offered by edX. This type of verification is useful for professional education/certifications certificate_type_description = _("A {cert_type} certificate signifies that a " "learner has agreed to abide by the honor code established by " "{platform_name} and has completed all of the required tasks for this course " - "under its guidelines. A {cert_type} certificate also indicates that the " - "identity of the learner has been checked and " - "is valid.").format(cert_type=certificate_type, - platform_name=platform_name) + "under its guidelines. ").format(cert_type=certificate_type, + platform_name=platform_name) + if settings.FEATURES.get('ENABLE_CERTIFICATES_IDV_REQUIREMENT'): + certificate_type_description += _("A {cert_type} certificate also indicates that the " + "identity of the learner has been checked and " + "is valid.").format(cert_type=certificate_type) + elif mode == 'xseries': # Translators: This text describes the 'XSeries' course certificate type. An XSeries is a collection of # courses related to each other in a meaningful way, such as a specific topic or theme, or even an organization @@ -247,7 +252,10 @@ def _update_course_context(request, context, course, platform_name): context['accomplishment_copy_course_name'] = accomplishment_copy_course_name course_number = course.display_coursenumber if course.display_coursenumber else course.number context['course_number'] = course_number - if context['organization_long_name']: + course_description_override = context['certificate_data'].get('course_description', '') + if course_description_override: + context['accomplishment_copy_course_description'] = course_description_override + elif context['organization_long_name']: # Translators: This text represents the description of course context['accomplishment_copy_course_description'] = _('a course of study offered by {partner_short_name}, ' 'an online learning initiative of ' @@ -505,7 +513,7 @@ def render_cert_by_uuid(request, certificate_uuid): test_func=lambda request: request.GET.get('preview', None) ) @pluggable_override('OVERRIDE_RENDER_CERTIFICATE_VIEW') -def render_html_view(request, course_id, certificate=None): +def render_html_view(request, course_id, certificate=None): # pylint: disable=too-many-statements """ This public view generates an HTML representation of the specified user and course If a certificate is not available, we display a "Sorry!" screen instead From 388063f22a9840b64445e12283b632901b948c43 Mon Sep 17 00:00:00 2001 From: Farhaan Bukhsh <farhaan@opencraft.com> Date: Mon, 25 Apr 2022 23:06:41 +0200 Subject: [PATCH 16/20] feat: Added date configuration to Schedule & Details settings page This feature help to configure the date formatt in Schedule and Details settings page. Signed-off-by: Farhaan Bukhsh <farhaan@opencraft.com> Co-authored-by: Joseph Curtin <jbcurtin@opencraft.com> --- cms/djangoapps/contentstore/views/course.py | 8 ++++++++ cms/envs/common.py | 5 +++++ cms/static/js/utils/date_utils.js | 7 ++++++- cms/templates/settings.html | 13 +++++++------ 4 files changed, 26 insertions(+), 7 deletions(-) diff --git a/cms/djangoapps/contentstore/views/course.py b/cms/djangoapps/contentstore/views/course.py index f559a0b854c1..54a6c15dd662 100644 --- a/cms/djangoapps/contentstore/views/course.py +++ b/cms/djangoapps/contentstore/views/course.py @@ -1158,6 +1158,13 @@ def settings_handler(request, course_key_string): # lint-amnesty, pylint: disab verified_mode = CourseMode.verified_mode_for_course(course_key, include_expired=True) upgrade_deadline = (verified_mode and verified_mode.expiration_datetime and verified_mode.expiration_datetime.isoformat()) + + date_placeholder_format = configuration_helpers.get_value_for_org( + course_module.location.org, + 'SCHEDULE_DETAIL_FORMAT', + settings.SCHEDULE_DETAIL_FORMAT + ).upper() + settings_context = { 'context_course': course_module, 'course_locator': course_key, @@ -1182,6 +1189,7 @@ def settings_handler(request, course_key_string): # lint-amnesty, pylint: disab 'enable_extended_course_details': enable_extended_course_details, 'upgrade_deadline': upgrade_deadline, 'mfe_proctored_exam_settings_url': get_proctored_exam_settings_url(course_module.id), + 'date_placeholder_format': date_placeholder_format, } if is_prerequisite_courses_enabled(): courses, in_process_course_actions = get_courses_accessible_to_user(request) diff --git a/cms/envs/common.py b/cms/envs/common.py index d69043abf966..0f0133b1f62c 100644 --- a/cms/envs/common.py +++ b/cms/envs/common.py @@ -175,6 +175,11 @@ # templates. STUDIO_NAME = _("Your Platform Studio") STUDIO_SHORT_NAME = _("Studio") + +# .. setting_name: SCHEDULE_DETAIL_FORMAT +# .. setting_default: MM/DD/YYYY' +# .. setting_description: Settings to configure the date format in Schedule & Details page +SCHEDULE_DETAIL_FORMAT = 'MM/DD/YYYY' FEATURES = { 'GITHUB_PUSH': False, diff --git a/cms/static/js/utils/date_utils.js b/cms/static/js/utils/date_utils.js index 26e2c52e86cf..540eaca6d42a 100644 --- a/cms/static/js/utils/date_utils.js +++ b/cms/static/js/utils/date_utils.js @@ -145,7 +145,12 @@ function($, date, TriggerChangeEventOnEnter, moment) { // instrument as date and time pickers timefield.timepicker({timeFormat: 'H:i'}); - datefield.datepicker(); + var placeholder = datefield.attr('placeholder'); + if (placeholder == 'DD/MM/YYYY') { + datefield.datepicker({dateFormat: 'dd/mm/yy'}); + } else { + datefield.datepicker(); + } // Using the change event causes setfield to be triggered twice, but it is necessary // to pick up when the date is typed directly in the field. diff --git a/cms/templates/settings.html b/cms/templates/settings.html index 965578270968..09549684b6e6 100644 --- a/cms/templates/settings.html +++ b/cms/templates/settings.html @@ -223,7 +223,7 @@ <h2 class="title-2">${_('Course Schedule')}</h2> <li class="field-group field-group-course-start" id="course-start"> <div class="field date" id="field-course-start-date"> <label for="course-start-date">${_("Course Start Date")}</label> - <input type="text" class="start-date date start datepicker" id="course-start-date" placeholder="MM/DD/YYYY" autocomplete="off" /> + <input type="text" class="start-date date start datepicker" id="course-start-date" placeholder="${date_placeholder_format}" autocomplete="off" /> <span class="icon icon-inline fa fa-calendar-check-o datepicker-icon" aria-hidden="true"></span> <span class="tip tip-stacked">${_("First day the course begins")}</span> </div> @@ -238,7 +238,7 @@ <h2 class="title-2">${_('Course Schedule')}</h2> <li class="field-group field-group-course-end" id="course-end"> <div class="field date" id="field-course-end-date"> <label for="course-end-date">${_("Course End Date")}</label> - <input type="text" class="end-date date end" id="course-end-date" placeholder="MM/DD/YYYY" autocomplete="off" /> + <input type="text" class="end-date date end" id="course-end-date" placeholder="${date_placeholder_format}" autocomplete="off" /> <span class="icon icon-inline fa fa-calendar-check-o datepicker-icon" aria-hidden="true"></span> <span class="tip tip-stacked">${_("Last day your course is active")}</span> </div> @@ -304,8 +304,9 @@ <h2 class="title-2">${_('Course Schedule')}</h2> <div class="field date" id="field-certificate-available-date" > % endif <label for="certificate-available-date">${_("Certificates Available Date")}</label> - <input type="text" class="certificate-available-date date start datepicker" id="certificate-available-date" placeholder="MM/DD/YYYY" autocomplete="off" /> + <input type="text" class="certificate-available-date date start datepicker" id="certificate-available-date" placeholder="${date_placeholder_format}" autocomplete="off" /> <span class="icon icon-inline fa fa-calendar-check-o datepicker-icon" aria-hidden="true"></span> + <span class="tip tip-stacked">${_("By default, 48 hours after course end date")}</span> </div> </li> </ol> @@ -315,7 +316,7 @@ <h2 class="title-2">${_('Course Schedule')}</h2> <li class="field-group field-group-enrollment-start" id="enrollment-start"> <div class="field date" id="field-enrollment-start-date"> <label for="course-enrollment-start-date">${_("Enrollment Start Date")}</label> - <input type="text" class="start-date date start" id="course-enrollment-start-date" placeholder="MM/DD/YYYY" autocomplete="off" /> + <input type="text" class="start-date date start" id="course-enrollment-start-date" placeholder="${date_placeholder_format}" autocomplete="off" /> <span class="icon icon-inline fa fa-calendar-check-o datepicker-icon" aria-hidden="true"></span> <span class="tip tip-stacked">${_("First day students can enroll")}</span> </div> @@ -333,7 +334,7 @@ <h2 class="title-2">${_('Course Schedule')}</h2> <li class="field-group field-group-enrollment-end" id="enrollment-end"> <div class="field date ${enrollment_end_editable_class}" id="field-enrollment-end-date"> <label for="course-enrollment-end-date">${_("Enrollment End Date")}</label> - <input type="text" class="end-date date end" id="course-enrollment-end-date" placeholder="MM/DD/YYYY" autocomplete="off" ${enrollment_end_readonly} /> + <input type="text" class="end-date date end" id="course-enrollment-end-date" placeholder="${date_placeholder_format}" autocomplete="off" ${enrollment_end_readonly} /> <span class="icon icon-inline fa fa-calendar-check-o datepicker-icon" aria-hidden="true"></span> <span class="tip tip-stacked"> ${_("Last day students can enroll.")} @@ -356,7 +357,7 @@ <h2 class="title-2">${_('Course Schedule')}</h2> <li class="field-group field-group-upgrade-deadline" id="upgrade-deadline"> <div class="field date is-not-editable" id="field-upgrade-deadline-date"> <label for="course-upgrade-deadline-date">${_("Upgrade Deadline Date")}</label> - <input type="text" class="date upgrade-deadline" id="course-upgrade-deadline-date" placeholder="MM/DD/YYYY" autocomplete="off" readonly aria-readonly="true" /> + <input type="text" class="date upgrade-deadline" id="course-upgrade-deadline-date" placeholder="${date_placeholder_format}" autocomplete="off" readonly aria-readonly="true" /> <span class="tip tip-stacked"> ${_("Last day students can upgrade to a verified enrollment.")} ${_("Contact your {platform_name} partner manager to update these settings.").format(platform_name=settings.PLATFORM_NAME)} From d4522f100f704e5e15bddc7031dac28ef0e1b579 Mon Sep 17 00:00:00 2001 From: Soban Javed <iamsobanjaved@gmail.com> Date: Tue, 7 Dec 2021 17:44:52 +0500 Subject: [PATCH 17/20] test: save pytest warning reports to GHA artifacts - add pytest_hooks import back in common/lib/conftest.py. This import was removed during refactoring work (cherry picked from commit b4ac8d284ab22c2167560e00adbfc6e8a3dfed15) --- .github/workflows/unit-tests.yml | 44 ++++++++++++++++++++++++++++++++ common/lib/conftest.py | 3 +++ openedx/core/process_warnings.py | 2 +- 3 files changed, 48 insertions(+), 1 deletion(-) diff --git a/.github/workflows/unit-tests.yml b/.github/workflows/unit-tests.yml index 438ab7c8e9c6..624636078044 100644 --- a/.github/workflows/unit-tests.yml +++ b/.github/workflows/unit-tests.yml @@ -76,3 +76,47 @@ jobs: - name: run tests run: | python -Wd -m pytest -p no:randomly --ds=${{ env.settings_path }} ${{ env.unit_test_paths }} + + - name: rename warnings json file + if: success() + run: | + cd test_root/log + mv pytest_warnings.json pytest_warnings_${{ matrix.shard_name }}.json + + - name: save pytest warnings json file + if: success() + uses: actions/upload-artifact@v2 + with: + name: pytest-warnings-json + path: | + test_root/log/pytest_warnings*.json + + compile-warnings-report: + runs-on: [ self-hosted ] + needs: [ run-tests ] + steps: + - name: sync directory owner + run: sudo chown runner:runner -R .* + - uses: actions/checkout@v2 + - name: collect pytest warnings files + uses: actions/download-artifact@v2 + with: + name: pytest-warnings-json + path: test_root/log + + - name: display structure of downloaded files + run: ls -la test_root/log + + - name: compile warnings report + run: | + python openedx/core/process_warnings.py --dir-path test_root/log --html-path reports/pytest_warnings/warning_report_all.html + + - name: save warning report + if: success() + uses: actions/upload-artifact@v2 + with: + name: pytest-warning-report-html + path: | + reports/pytest_warnings/warning_report_all.html + + diff --git a/common/lib/conftest.py b/common/lib/conftest.py index 9fd136d28fdc..c3af3f60914e 100644 --- a/common/lib/conftest.py +++ b/common/lib/conftest.py @@ -7,6 +7,9 @@ from safe_lxml import defuse_xml_libs +# This import is needed for pytest plugin configuration, so please avoid deleting this during refactoring +from openedx.core.pytest_hooks import pytest_configure # pylint: disable=unused-import + defuse_xml_libs() diff --git a/openedx/core/process_warnings.py b/openedx/core/process_warnings.py index 6141fc874dcb..4cdbe59f3c3a 100644 --- a/openedx/core/process_warnings.py +++ b/openedx/core/process_warnings.py @@ -91,7 +91,7 @@ def read_warning_data(dir_path): # TODO(jinder): currently this is hard-coded in, maybe create a constants file with info # THINK(jinder): but creating file for one constant seems overkill warnings_file_name_regex = ( - r"pytest_warnings_?\d*\.json" # noqa pylint: disable=W1401 + r"pytest_warnings_?[\w-]*\.json" # noqa pylint: disable=W1401 ) # iterate through files_in_dir and see if they match our know file name pattern From 90420d9f39c92b25cfc3bd9120631b959e3cde4a Mon Sep 17 00:00:00 2001 From: Muhammad Soban Javed <58461728+iamsobanjaved@users.noreply.github.com> Date: Fri, 21 Jan 2022 07:58:57 +0100 Subject: [PATCH 18/20] test: run openedx and common tests with both lms and cms settings (#29676) (cherry picked from commit 4e22a38ca5db3874afbc6f0abe526f82832d331b) --- .github/workflows/unit-test-shards.json | 283 +++++++++++++++++- .github/workflows/unit-tests.yml | 9 +- .../workflows/verify-gha-unit-tests-count.yml | 6 +- scripts/gha-shards-readme.md | 35 +++ scripts/gha_unit_tests_collector.py | 41 +-- scripts/unit_test_shards_parser.py | 32 +- 6 files changed, 358 insertions(+), 48 deletions(-) create mode 100644 scripts/gha-shards-readme.md diff --git a/.github/workflows/unit-test-shards.json b/.github/workflows/unit-test-shards.json index 02b91633db2c..edd07103ce6d 100644 --- a/.github/workflows/unit-test-shards.json +++ b/.github/workflows/unit-test-shards.json @@ -1,14 +1,273 @@ { - "lms-1": "lms/djangoapps/badges/ lms/djangoapps/branding/ lms/djangoapps/bulk_email/ lms/djangoapps/bulk_enroll/ lms/djangoapps/bulk_user_retirement/ lms/djangoapps/ccx/ lms/djangoapps/certificates/ lms/djangoapps/commerce/", - "lms-2": "lms/djangoapps/course_api/ lms/djangoapps/course_blocks/ lms/djangoapps/course_goals/ lms/djangoapps/course_home_api/ lms/djangoapps/course_wiki/ lms/djangoapps/coursewarehistoryextended/ lms/djangoapps/debug/", - "lms-3": "lms/djangoapps/courseware/", - "lms-4": "lms/djangoapps/discussion/ lms/djangoapps/edxnotes/ lms/djangoapps/email_marketing/ lms/djangoapps/experiments/", - "lms-5": "lms/djangoapps/gating/ lms/djangoapps/grades/ lms/djangoapps/instructor/ lms/djangoapps/instructor_analytics/", - "lms-6": "lms/djangoapps/instructor_task/ lms/djangoapps/learner_dashboard/ lms/djangoapps/lms_initialization/ lms/djangoapps/lms_xblock/ lms/djangoapps/lti_provider/ lms/djangoapps/mailing/ lms/djangoapps/mobile_api/ lms/djangoapps/monitoring/ lms/djangoapps/program_enrollments/ lms/djangoapps/rss_proxy/ lms/djangoapps/static_template_view/ lms/djangoapps/staticbook/ lms/djangoapps/support/ lms/djangoapps/survey/ lms/djangoapps/teams/ lms/djangoapps/tests/ lms/djangoapps/verify_student/ lms/envs/ lms/lib/ lms/tests.py", - "openedx-1": "openedx/core/djangoapps/ace_common/ openedx/core/djangoapps/cors_csrf/ openedx/core/djangoapps/agreements/ openedx/core/djangoapps/api_admin/ openedx/core/djangoapps/auth_exchange/ openedx/core/djangoapps/bookmarks/ openedx/core/djangoapps/cache_toolbox/ openedx/core/djangoapps/catalog/ openedx/core/djangoapps/ccxcon/ openedx/core/djangoapps/commerce/ openedx/core/djangoapps/common_initialization/ openedx/core/djangoapps/common_views/ openedx/core/djangoapps/config_model_utils/ openedx/core/djangoapps/content/ openedx/core/djangoapps/content_libraries/ openedx/core/djangoapps/contentserver/ openedx/core/djangoapps/cookie_metadata/ openedx/core/djangoapps/course_apps/ openedx/core/djangoapps/course_date_signals/ openedx/core/djangoapps/course_groups/ openedx/core/djangoapps/coursegraph/ openedx/core/djangoapps/courseware_api/ openedx/core/djangoapps/crawlers/ openedx/core/djangoapps/credentials/ openedx/core/djangoapps/credit/ openedx/core/djangoapps/dark_lang/ openedx/core/djangoapps/debug/ openedx/core/djangoapps/demographics/ openedx/core/djangoapps/discussions/ openedx/core/djangoapps/django_comment_common/ openedx/core/djangoapps/embargo/ openedx/core/djangoapps/enrollments/ openedx/core/djangoapps/external_user_ids/", - "openedx-2": "openedx/core/djangoapps/geoinfo/ openedx/core/djangoapps/header_control/ openedx/core/djangoapps/heartbeat/ openedx/core/djangoapps/lang_pref/ openedx/core/djangoapps/models/ openedx/core/djangoapps/monkey_patch/ openedx/core/djangoapps/oauth_dispatch/ openedx/core/djangoapps/olx_rest_api/ openedx/core/djangoapps/password_policy/ openedx/core/djangoapps/plugin_api/ openedx/core/djangoapps/plugins/ openedx/core/djangoapps/profile_images/ openedx/core/djangoapps/programs/ openedx/core/djangoapps/safe_sessions/ openedx/core/djangoapps/schedules/ openedx/core/djangoapps/self_paced/ openedx/core/djangoapps/service_status/ openedx/core/djangoapps/session_inactivity_timeout/ openedx/core/djangoapps/signals/ openedx/core/djangoapps/site_configuration/ openedx/core/djangoapps/system_wide_roles/ openedx/core/djangoapps/theming/ openedx/core/djangoapps/user_api/ openedx/core/djangoapps/user_authn/ openedx/core/djangoapps/util/ openedx/core/djangoapps/verified_track_content/ openedx/core/djangoapps/video_config/ openedx/core/djangoapps/video_pipeline/ openedx/core/djangoapps/waffle_utils/ openedx/core/djangoapps/xblock/ openedx/core/djangoapps/xmodule_django/ openedx/core/djangoapps/zendesk_proxy/ openedx/core/djangolib/ openedx/core/lib/ openedx/core/tests/ openedx/features/ openedx/testing/ openedx/tests/", - "cms-1": "cms/djangoapps/api/ cms/djangoapps/cms_user_tasks/ cms/djangoapps/course_creators/ cms/djangoapps/export_course_metadata/ cms/djangoapps/maintenance/ cms/djangoapps/models/ cms/djangoapps/pipeline_js/ cms/djangoapps/xblock_config/ cms/envs/ cms/lib/", - "cms-2": "cms/djangoapps/contentstore/", - "common-1": "common/djangoapps/", - "common-2": "common/lib/" + "lms-1": { + "settings": "lms.envs.test", + "paths": [ + "lms/djangoapps/badges/", + "lms/djangoapps/branding/", + "lms/djangoapps/bulk_email/", + "lms/djangoapps/bulk_enroll/", + "lms/djangoapps/bulk_user_retirement/", + "lms/djangoapps/ccx/", + "lms/djangoapps/certificates/", + "lms/djangoapps/commerce/" + ] + }, + "lms-2": { + "settings": "lms.envs.test", + "paths": [ + "lms/djangoapps/course_api/", + "lms/djangoapps/course_blocks/", + "lms/djangoapps/course_goals/", + "lms/djangoapps/course_home_api/", + "lms/djangoapps/course_wiki/", + "lms/djangoapps/coursewarehistoryextended/", + "lms/djangoapps/debug/" + ] + }, + "lms-3": { + "settings": "lms.envs.test", + "paths": [ + "lms/djangoapps/courseware/" + ] + }, + "lms-4": { + "settings": "lms.envs.test", + "paths": [ + "lms/djangoapps/discussion/", + "lms/djangoapps/edxnotes/", + "lms/djangoapps/email_marketing/", + "lms/djangoapps/experiments/" + ] + }, + "lms-5": { + "settings": "lms.envs.test", + "paths": [ + "lms/djangoapps/gating/", + "lms/djangoapps/grades/", + "lms/djangoapps/instructor/", + "lms/djangoapps/instructor_analytics/" + ] + }, + "lms-6": { + "settings": "lms.envs.test", + "paths": [ + "lms/djangoapps/instructor_task/", + "lms/djangoapps/learner_dashboard/", + "lms/djangoapps/lms_initialization/", + "lms/djangoapps/lms_xblock/", + "lms/djangoapps/lti_provider/", + "lms/djangoapps/mailing/", + "lms/djangoapps/mobile_api/", + "lms/djangoapps/monitoring/", + "lms/djangoapps/program_enrollments/", + "lms/djangoapps/rss_proxy/", + "lms/djangoapps/static_template_view/", + "lms/djangoapps/staticbook/", + "lms/djangoapps/support/", + "lms/djangoapps/survey/", + "lms/djangoapps/teams/", + "lms/djangoapps/tests/", + "lms/djangoapps/verify_student/", + "lms/envs/", + "lms/lib/", + "lms/tests.py" + ] + }, + "openedx-1": { + "settings": "lms.envs.test", + "paths": [ + "openedx/core/djangoapps/ace_common/", + "openedx/core/djangoapps/cors_csrf/", + "openedx/core/djangoapps/agreements/", + "openedx/core/djangoapps/api_admin/", + "openedx/core/djangoapps/auth_exchange/", + "openedx/core/djangoapps/bookmarks/", + "openedx/core/djangoapps/cache_toolbox/", + "openedx/core/djangoapps/catalog/", + "openedx/core/djangoapps/ccxcon/", + "openedx/core/djangoapps/commerce/", + "openedx/core/djangoapps/common_initialization/", + "openedx/core/djangoapps/common_views/", + "openedx/core/djangoapps/config_model_utils/", + "openedx/core/djangoapps/content/", + "openedx/core/djangoapps/content_libraries/", + "openedx/core/djangoapps/contentserver/", + "openedx/core/djangoapps/cookie_metadata/", + "openedx/core/djangoapps/course_apps/", + "openedx/core/djangoapps/course_date_signals/", + "openedx/core/djangoapps/course_groups/", + "openedx/core/djangoapps/coursegraph/", + "openedx/core/djangoapps/courseware_api/", + "openedx/core/djangoapps/crawlers/", + "openedx/core/djangoapps/credentials/", + "openedx/core/djangoapps/credit/", + "openedx/core/djangoapps/dark_lang/", + "openedx/core/djangoapps/debug/", + "openedx/core/djangoapps/demographics/", + "openedx/core/djangoapps/discussions/", + "openedx/core/djangoapps/django_comment_common/", + "openedx/core/djangoapps/embargo/", + "openedx/core/djangoapps/enrollments/", + "openedx/core/djangoapps/external_user_ids/" + ] + }, + "openedx-2": { + "settings": "lms.envs.test", + "paths": [ + "openedx/core/djangoapps/geoinfo/", + "openedx/core/djangoapps/header_control/", + "openedx/core/djangoapps/heartbeat/", + "openedx/core/djangoapps/lang_pref/", + "openedx/core/djangoapps/models/", + "openedx/core/djangoapps/monkey_patch/", + "openedx/core/djangoapps/oauth_dispatch/", + "openedx/core/djangoapps/olx_rest_api/", + "openedx/core/djangoapps/password_policy/", + "openedx/core/djangoapps/plugin_api/", + "openedx/core/djangoapps/plugins/", + "openedx/core/djangoapps/profile_images/", + "openedx/core/djangoapps/programs/", + "openedx/core/djangoapps/safe_sessions/", + "openedx/core/djangoapps/schedules/", + "openedx/core/djangoapps/self_paced/", + "openedx/core/djangoapps/service_status/", + "openedx/core/djangoapps/session_inactivity_timeout/", + "openedx/core/djangoapps/signals/", + "openedx/core/djangoapps/site_configuration/", + "openedx/core/djangoapps/system_wide_roles/", + "openedx/core/djangoapps/theming/", + "openedx/core/djangoapps/user_api/", + "openedx/core/djangoapps/user_authn/", + "openedx/core/djangoapps/util/", + "openedx/core/djangoapps/verified_track_content/", + "openedx/core/djangoapps/video_config/", + "openedx/core/djangoapps/video_pipeline/", + "openedx/core/djangoapps/waffle_utils/", + "openedx/core/djangoapps/xblock/", + "openedx/core/djangoapps/xmodule_django/", + "openedx/core/djangoapps/zendesk_proxy/", + "openedx/core/djangolib/", + "openedx/core/lib/", + "openedx/core/tests/", + "openedx/features/", + "openedx/testing/", + "openedx/tests/" + ] + }, + "openedx-3": { + "settings": "cms.envs.test", + "paths": [ + "openedx/core/djangoapps/ace_common/", + "openedx/core/djangoapps/cors_csrf/", + "openedx/core/djangoapps/agreements/", + "openedx/core/djangoapps/api_admin/", + "openedx/core/djangoapps/auth_exchange/", + "openedx/core/djangoapps/bookmarks/", + "openedx/core/djangoapps/cache_toolbox/", + "openedx/core/djangoapps/catalog/", + "openedx/core/djangoapps/ccxcon/", + "openedx/core/djangoapps/commerce/", + "openedx/core/djangoapps/common_initialization/", + "openedx/core/djangoapps/common_views/", + "openedx/core/djangoapps/config_model_utils/", + "openedx/core/djangoapps/content/", + "openedx/core/djangoapps/content_libraries/", + "openedx/core/djangoapps/contentserver/", + "openedx/core/djangoapps/cookie_metadata/", + "openedx/core/djangoapps/course_apps/", + "openedx/core/djangoapps/course_date_signals/", + "openedx/core/djangoapps/course_groups/", + "openedx/core/djangoapps/coursegraph/", + "openedx/core/djangoapps/courseware_api/", + "openedx/core/djangoapps/crawlers/", + "openedx/core/djangoapps/credentials/", + "openedx/core/djangoapps/credit/", + "openedx/core/djangoapps/dark_lang/", + "openedx/core/djangoapps/debug/", + "openedx/core/djangoapps/demographics/", + "openedx/core/djangoapps/discussions/", + "openedx/core/djangoapps/django_comment_common/", + "openedx/core/djangoapps/embargo/", + "openedx/core/djangoapps/enrollments/", + "openedx/core/djangoapps/external_user_ids/" + ] + }, + "openedx-4": { + "settings": "cms.envs.test", + "paths": [ + "openedx/core/djangoapps/geoinfo/", + "openedx/core/djangoapps/header_control/", + "openedx/core/djangoapps/heartbeat/", + "openedx/core/djangoapps/lang_pref/", + "openedx/core/djangoapps/models/", + "openedx/core/djangoapps/monkey_patch/", + "openedx/core/djangoapps/oauth_dispatch/", + "openedx/core/djangoapps/olx_rest_api/", + "openedx/core/djangoapps/password_policy/", + "openedx/core/djangoapps/plugin_api/", + "openedx/core/djangoapps/plugins/", + "openedx/core/djangoapps/profile_images/", + "openedx/core/djangoapps/programs/", + "openedx/core/djangoapps/safe_sessions/", + "openedx/core/djangoapps/schedules/", + "openedx/core/djangoapps/self_paced/", + "openedx/core/djangoapps/service_status/", + "openedx/core/djangoapps/session_inactivity_timeout/", + "openedx/core/djangoapps/signals/", + "openedx/core/djangoapps/site_configuration/", + "openedx/core/djangoapps/system_wide_roles/", + "openedx/core/djangoapps/theming/", + "openedx/core/djangoapps/user_api/", + "openedx/core/djangoapps/user_authn/", + "openedx/core/djangoapps/util/", + "openedx/core/djangoapps/verified_track_content/", + "openedx/core/djangoapps/video_config/", + "openedx/core/djangoapps/video_pipeline/", + "openedx/core/djangoapps/waffle_utils/", + "openedx/core/djangoapps/xblock/", + "openedx/core/djangoapps/xmodule_django/", + "openedx/core/djangoapps/zendesk_proxy/", + "openedx/core/lib/", + "openedx/tests/" + ] + }, + "cms-1": { + "settings": "cms.envs.test", + "paths": [ + "cms/djangoapps/api/", + "cms/djangoapps/cms_user_tasks/", + "cms/djangoapps/course_creators/", + "cms/djangoapps/export_course_metadata/", + "cms/djangoapps/maintenance/", + "cms/djangoapps/models/", + "cms/djangoapps/pipeline_js/", + "cms/djangoapps/xblock_config/", + "cms/envs/", + "cms/lib/" + ] + }, + "cms-2": { + "settings": "cms.envs.test", + "paths": [ + "cms/djangoapps/contentstore/" + ] + }, + "common-1": { + "settings": "lms.envs.test", + "paths": [ + "common/djangoapps/" + ] + }, + "common-2": { + "settings": "lms.envs.test", + "paths": [ + "common/lib/" + ] + }, + "common-3": { + "settings": "cms.envs.test", + "paths": [ + "common/djangoapps/" + ] + } } diff --git a/.github/workflows/unit-tests.yml b/.github/workflows/unit-tests.yml index 624636078044..ae5b47c8bf6c 100644 --- a/.github/workflows/unit-tests.yml +++ b/.github/workflows/unit-tests.yml @@ -25,10 +25,13 @@ jobs: "lms-6", "openedx-1", "openedx-2", + "openedx-3", + "openedx-4", "cms-1", "cms-2", "common-1", "common-2", + "common-3", ] @@ -43,13 +46,9 @@ jobs: sudo chmod -R a+rw /data/db mongod & - - name: set top-level module name - run: | - echo "module_name=$(echo '${{ matrix.shard_name }}' | awk -F '-' '{print $1}')" >> $GITHUB_ENV - - name: set settings path run: | - echo "settings_path=$(if [ '${{ env.module_name }}' = 'cms' ]; then echo 'cms.envs.test'; else echo 'lms.envs.test' ; fi)" >> $GITHUB_ENV + echo "settings_path=$(python scripts/unit_test_shards_parser.py --shard-name=${{ matrix.shard_name }} --output settings )" >> $GITHUB_ENV # - name: set pytest randomly option # run: | diff --git a/.github/workflows/verify-gha-unit-tests-count.yml b/.github/workflows/verify-gha-unit-tests-count.yml index 4f9aafbdde2c..7e5950aa94db 100644 --- a/.github/workflows/verify-gha-unit-tests-count.yml +++ b/.github/workflows/verify-gha-unit-tests-count.yml @@ -50,8 +50,10 @@ jobs: echo All root unit tests count: ${{ env.root_all_unit_tests_count }} echo All shards unit tests count: ${{ env.shards_all_unit_tests_count }} - - name: verify unit tests count + - name: fail the check if: ${{ env.root_all_unit_tests_count != env.shards_all_unit_tests_count }} run: | - echo "::error title='Unit test modules in unit-test-shards.json (unit-tests.yml workflow) are outdated'::unit tests running in unit-tests workflow don't match the count for unit tests for entire edx-platform suite, please update the unit-test-shards.json under .github/workflows to add any missing apps and match the count" + echo "::error title='Unit test modules in unit-test-shards.json (unit-tests.yml workflow) are outdated'::unit tests running in unit-tests + workflow don't match the count for unit tests for entire edx-platform suite, please update the unit-test-shards.json under .github/workflows + to add any missing apps and match the count. for more details please take a look at scripts/gha-shards-readme.md" exit 1 diff --git a/scripts/gha-shards-readme.md b/scripts/gha-shards-readme.md new file mode 100644 index 000000000000..8a3d96da3a37 --- /dev/null +++ b/scripts/gha-shards-readme.md @@ -0,0 +1,35 @@ +# Unit tests sharding strategy + +#### background +Unit tests are run in parallel (in GitHub Actions matrices) using the sharding strategy specified in unit-test-shards.json +We've divided the top level modules into multiple shards to achieve better parallelism. +The configuration in unit-test-shards.json specifies the shard name as key for each shard and the value contains an object +with django settings for each module and paths for submodules to test for example: +```json +{ + "lms-1": { + "paths": ["lms/djangoapps/course_api", ...], + "settings": "lms.envs.test", + } + . + . + . +} +``` +The `common` and `openedx` modules are tested with both `lms` and `cms` settings; that's why there are shards with the same `openedx` +submodules but with different Django settings. +For more details on sharding strategy please refer to this section on [sharding](https://openedx.atlassian.net/wiki/spaces/AT/pages/3235971586/edx-platfrom+unit+tests+migration+from+Jenkins+to+Github+Actions#Motivation-for-sharding-manually) + +#### Unit tests count check is failing +There's a check in place that makes sure that all the unit tests under edx-platform modules are specified in `unit-test-shards.json` +If there's a mismatch between the number of unit tests collected from `unit-test-shards.json` and the number of unit tests collected +against the entire codebase the check will fail. +You'd have to update the `unit-test-shards.json` file manually to fix this. + +##### How to fix +- If you've added a new django app to the codebase, and you want to add it to the unit tests you need to add it to the `unit-test-shards.json`, details on where (in which shard) to place your Django app please refer to the [sharding](https://openedx.atlassian.net/wiki/spaces/AT/pages/3235971586/edx-platfrom+unit+tests+migration+from+Jenkins+to+Github+Actions#Where-should-I-place-my-new-Django-app) section in this document. +- If you haven't added any new django app to the codebase, you can debug / verify this by collecting unit tests against a submodule by running `pytest` for example: +``` +pytest --collect-only --ds=cms.envs.test cms/ +``` +For more details on how this check collects and compares the unit tests count please take a look at [verify unit tests count](../.github/workflows/verify-gha-unit-tests-count.yml) diff --git a/scripts/gha_unit_tests_collector.py b/scripts/gha_unit_tests_collector.py index 67366211edfa..b72257049e05 100644 --- a/scripts/gha_unit_tests_collector.py +++ b/scripts/gha_unit_tests_collector.py @@ -1,41 +1,44 @@ -import sys -import os -import yaml import argparse import json +import sys def get_all_unit_test_shards(): - unit_tests_json = f'{os.getcwd()}/.github/workflows/unit-test-shards.json' + unit_tests_json = '.github/workflows/unit-test-shards.json' with open(unit_tests_json) as file: - unit_test_workflow_shards = json.loads(file.read()) + unit_test_workflow_shards = json.load(file) return unit_test_workflow_shards -def get_modules_except_cms(): - all_unit_test_shards = get_all_unit_test_shards() - return [paths for shard_name, paths in all_unit_test_shards.items() if not paths.startswith('cms')] +def update_unit_test_modules(module_name, shard_config, unit_test_modules): + is_cms_shard_path = shard_config['paths'][0].startswith('cms') + if is_cms_shard_path and module_name == "cms": + unit_test_modules.update(shard_config.get('paths')) + elif not is_cms_shard_path and module_name != "cms": + unit_test_modules.update(shard_config.get('paths')) + return unit_test_modules -def get_cms_modules(): + +def get_unit_test_modules(module_name="lms"): + unit_test_modules = set() all_unit_test_shards = get_all_unit_test_shards() - return [paths for shard_name, paths in all_unit_test_shards.items() if paths.startswith('cms')] + for shard_name, shard_config in all_unit_test_shards.items(): + unit_test_modules = update_unit_test_modules(module_name, shard_config, unit_test_modules) + return unit_test_modules if __name__ == "__main__": parser = argparse.ArgumentParser() parser.add_argument("--cms-only", action="store_true", default="") parser.add_argument("--lms-only", action="store_true", default="") - argument = parser.parse_args() - if argument.lms_only: - modules = get_modules_except_cms() - elif argument.cms_only: - modules = get_cms_modules() - else: - modules = get_all_unit_test_modules() + if not argument.cms_only and not argument.lms_only: + print("Please specify --cms-only or --lms-only") + sys.exit(1) - unit_test_paths = ' '.join(modules) - sys.stdout.write(unit_test_paths) + modules = get_unit_test_modules("cms") if argument.cms_only else get_unit_test_modules("lms") + paths_output = ' '.join(modules) + print(paths_output) diff --git a/scripts/unit_test_shards_parser.py b/scripts/unit_test_shards_parser.py index ce9b08074417..7cccb42c8938 100644 --- a/scripts/unit_test_shards_parser.py +++ b/scripts/unit_test_shards_parser.py @@ -1,27 +1,39 @@ -import sys -import os import argparse import json +import sys -def get_test_paths_for_shard(shard_name): - unit_tests_json = f'{os.getcwd()}/.github/workflows/unit-test-shards.json' +def load_unit_test_shards(shard_name): + unit_tests_json = '.github/workflows/unit-test-shards.json' with open(unit_tests_json) as file: - unit_test_workflow_shards = json.loads(file.read()) - + unit_test_workflow_shards = json.load(file) if shard_name not in unit_test_workflow_shards: sys.stdout.write("Error, invalid shard name provided. please provide a valid shard name as specified in unit-test-shards.json") + return unit_test_workflow_shards + + +def get_test_paths_for_shard(shard_name): + return load_unit_test_shards(shard_name).get(shard_name).get("paths") + + +def get_settings_for_shard(shard_name): + return load_unit_test_shards(shard_name).get(shard_name).get("settings") + - return unit_test_workflow_shards.get(shard_name) +def get_output(shard_name, output_argument): + if output_argument == "settings": + return get_settings_for_shard(shard_name) + return " ".join(get_test_paths_for_shard(shard_name)) if __name__ == "__main__": parser = argparse.ArgumentParser() parser.add_argument("--shard-name", action="store", default="") + parser.add_argument("--output", action="store", default="path", choices=["path", "settings"]) argument = parser.parse_args() if not argument.shard_name: - sys.stdout.write("Error, no shard name provided. please provide a valid shard name as specified in unit-test-shards.json") + sys.exit("Error, no shard name provided. please provide a valid shard name as specified in unit-test-shards.json") - unit_test_paths = get_test_paths_for_shard(argument.shard_name) - sys.stdout.write(unit_test_paths) + output = get_output(argument.shard_name, argument.output) + print(output) From cd1849d4f42ca677367ca39ba13e2ef35f1067cb Mon Sep 17 00:00:00 2001 From: Soban Javed <iamsobanjaved@gmail.com> Date: Wed, 16 Feb 2022 15:51:15 +0100 Subject: [PATCH 19/20] test: run unit-tests on forks - add verify unit test workflow also - use composite github action for syncing (cherry picked from commit 6ccadbbeb83a5ae95be65f42c02c3e2bd43de72e) --- .github/actions/unit-tests/action.yml | 34 ++++++ .github/actions/verify-tests-count/action.yml | 49 ++++++++ .github/workflows/unit-tests-gh-hosted.yml | 113 ++++++++++++++++++ .github/workflows/unit-tests.yml | 63 +++------- .../workflows/verify-gha-unit-tests-count.yml | 41 +------ 5 files changed, 218 insertions(+), 82 deletions(-) create mode 100644 .github/actions/unit-tests/action.yml create mode 100644 .github/actions/verify-tests-count/action.yml create mode 100644 .github/workflows/unit-tests-gh-hosted.yml diff --git a/.github/actions/unit-tests/action.yml b/.github/actions/unit-tests/action.yml new file mode 100644 index 000000000000..71ca72a7da11 --- /dev/null +++ b/.github/actions/unit-tests/action.yml @@ -0,0 +1,34 @@ +name: 'Run unit tests' +description: 'shared steps to run unit tests on both Github hosted and self hosted runners.' +runs: + using: "composite" + steps: + - name: set settings path + shell: bash + run: | + echo "settings_path=$(python scripts/unit_test_shards_parser.py --shard-name=${{ matrix.shard_name }} --output settings )" >> $GITHUB_ENV + + - name: get unit tests for shard + shell: bash + run: | + echo "unit_test_paths=$(python scripts/unit_test_shards_parser.py --shard-name=${{ matrix.shard_name }} )" >> $GITHUB_ENV + + - name: run tests + shell: bash + run: | + python -Wd -m pytest -p no:randomly --ds=${{ env.settings_path }} ${{ env.unit_test_paths }} + + - name: rename warnings json file + if: success() + shell: bash + run: | + cd test_root/log + mv pytest_warnings.json pytest_warnings_${{ matrix.shard_name }}.json + + - name: save pytest warnings json file + if: success() + uses: actions/upload-artifact@v2 + with: + name: pytest-warnings-json + path: | + test_root/log/pytest_warnings*.json diff --git a/.github/actions/verify-tests-count/action.yml b/.github/actions/verify-tests-count/action.yml new file mode 100644 index 000000000000..6357a4158c84 --- /dev/null +++ b/.github/actions/verify-tests-count/action.yml @@ -0,0 +1,49 @@ +name: 'Verify unit tests count' +description: 'shared steps to verify unit tests count on both Github hosted and self hosted runners.' +runs: + using: "composite" + steps: + - name: collect tests from all modules + shell: bash + run: | + echo "root_cms_unit_tests_count=$(pytest --collect-only --ds=cms.envs.test cms/ -q | head -n -2 | wc -l)" >> $GITHUB_ENV + echo "root_lms_unit_tests_count=$(pytest --collect-only --ds=lms.envs.test lms/ openedx/ common/djangoapps/ common/lib/ -q | head -n -2 | wc -l)" >> $GITHUB_ENV + + - name: get GHA unit test paths + shell: bash + run: | + echo "cms_unit_test_paths=$(python scripts/gha_unit_tests_collector.py --cms-only)" >> $GITHUB_ENV + echo "lms_unit_test_paths=$(python scripts/gha_unit_tests_collector.py --lms-only)" >> $GITHUB_ENV + + + - name: collect tests from GHA unit test shards + shell: bash + run: | + echo "cms_unit_tests_count=$(pytest --collect-only --ds=cms.envs.test ${{ env.cms_unit_test_paths }} -q | head -n -2 | wc -l)" >> $GITHUB_ENV + echo "lms_unit_tests_count=$(pytest --collect-only --ds=lms.envs.test ${{ env.lms_unit_test_paths }} -q | head -n -2 | wc -l)" >> $GITHUB_ENV + + + - name: add unit tests count + shell: bash + run: | + echo "root_all_unit_tests_count=$((${{ env.root_cms_unit_tests_count }}+${{ env.root_lms_unit_tests_count }}))" >> $GITHUB_ENV + echo "shards_all_unit_tests_count=$((${{ env.cms_unit_tests_count }}+${{ env.lms_unit_tests_count }}))" >> $GITHUB_ENV + + - name: print unit tests count + shell: bash + run: | + echo CMS unit tests from root: ${{ env.root_cms_unit_tests_count }} + echo LMS unit tests from root: ${{ env.root_lms_unit_tests_count }} + echo CMS unit tests from shards: ${{ env.cms_unit_tests_count }} + echo LMS unit tests from shards: ${{ env.lms_unit_tests_count }} + echo All root unit tests count: ${{ env.root_all_unit_tests_count }} + echo All shards unit tests count: ${{ env.shards_all_unit_tests_count }} + + - name: fail the check + shell: bash + if: ${{ env.root_all_unit_tests_count != env.shards_all_unit_tests_count }} + run: | + echo "::error title='Unit test modules in unit-test-shards.json (unit-tests.yml workflow) are outdated'::unit tests running in unit-tests + workflow don't match the count for unit tests for entire edx-platform suite, please update the unit-test-shards.json under .github/workflows + to add any missing apps and match the count. for more details please take a look at scripts/gha-shards-readme.md" + exit 1 diff --git a/.github/workflows/unit-tests-gh-hosted.yml b/.github/workflows/unit-tests-gh-hosted.yml new file mode 100644 index 000000000000..9fffd785544e --- /dev/null +++ b/.github/workflows/unit-tests-gh-hosted.yml @@ -0,0 +1,113 @@ +name: unit-tests-gh-hosted + +on: + pull_request: + push: + branches: + - master + - open-release/lilac.master + +jobs: + run-test: + if: github.repository != 'openedx/edx-platform' && github.repository != 'edx/edx-platform-private' + runs-on: ubuntu-20.04 + strategy: + fail-fast: false + matrix: + python-version: [ '3.8' ] + django-version: [ "3.2" ] + shard_name: [ + "lms-1", + "lms-2", + "lms-3", + "lms-4", + "lms-5", + "lms-6", + "openedx-1", + "openedx-2", + "openedx-3", + "openedx-4", + "cms-1", + "cms-2", + "common-1", + "common-2", + "common-3", + ] + name: gh-hosted-python-${{ matrix.python-version }},django-${{ matrix.django-version }},${{ matrix.shard_name }} + steps: + - uses: actions/checkout@v2 + + - name: Install Required System Packages + run: sudo apt-get update && sudo apt-get install libxmlsec1-dev lynx + + - name: Start MongoDB + uses: supercharge/mongodb-github-action@1.7.0 + with: + mongodb-version: 4.4 + + - name: Setup Python + uses: actions/setup-python@v2 + with: + python-version: ${{ matrix.python-version }} + + - name: Get pip cache dir + id: pip-cache-dir + run: | + echo "::set-output name=dir::$(pip cache dir)" + + - name: Cache pip dependencies + id: cache-dependencies + uses: actions/cache@v2 + with: + path: ${{ steps.pip-cache-dir.outputs.dir }} + key: ${{ runner.os }}-pip-${{ hashFiles('requirements/edx/development.txt') }} + restore-keys: ${{ runner.os }}-pip- + + - name: Install Required Python Dependencies + run: | + pip install -r requirements/pip.txt + pip install -r requirements/edx/development.txt --src ${{ runner.temp }} + pip install "django~=${{ matrix.django-version }}.0" + + - name: Setup and run tests + uses: ./.github/actions/unit-tests + + collect-and-verify: + if: github.repository != 'openedx/edx-platform' && github.repository != 'edx/edx-platform-private' + runs-on: ubuntu-20.04 + strategy: + matrix: + python-version: [ '3.8' ] + django-version: [ "3.2" ] + steps: + - uses: actions/checkout@v2 + + - name: Install Required System Packages + run: sudo apt-get update && sudo apt-get install libxmlsec1-dev + + - name: Setup Python + uses: actions/setup-python@v2 + with: + python-version: ${{ matrix.python-version }} + + - name: Get pip cache dir + id: pip-cache-dir + run: | + echo "::set-output name=dir::$(pip cache dir)" + + - name: Cache pip dependencies + id: cache-dependencies + uses: actions/cache@v2 + with: + path: ${{ steps.pip-cache-dir.outputs.dir }} + key: ${{ runner.os }}-pip-${{ hashFiles('requirements/edx/development.txt') }} + restore-keys: ${{ runner.os }}-pip- + + - name: Install Required Python Dependencies + run: | + pip install -r requirements/pip.txt + pip install -r requirements/edx/development.txt --src ${{ runner.temp }} + pip install "django~=${{ matrix.django-version }}.0" + + - name: verify unit tests count + uses: ./.github/actions/verify-tests-count diff --git a/.github/workflows/unit-tests.yml b/.github/workflows/unit-tests.yml index ae5b47c8bf6c..fb5602b7bedf 100644 --- a/.github/workflows/unit-tests.yml +++ b/.github/workflows/unit-tests.yml @@ -5,17 +5,15 @@ on: push: branches: - master - - open-release/*.master jobs: run-tests: + if: github.repository == 'openedx/edx-platform' || github.repository == 'edx/edx-platform-private' runs-on: [ edx-platform-runner ] strategy: matrix: python-version: ['3.8'] - django-version: - - "pinned" - #- "4.0" + django-version: ["3.2"] shard_name: [ "lms-1", "lms-2", @@ -39,59 +37,36 @@ jobs: steps: - name: sync directory owner run: sudo chown runner:runner -R .* - - uses: actions/checkout@v2 + + - name: checkout repo + uses: actions/checkout@v2 + + # This gives Mongo several chances to start. We started getting flakiness + # around 2022-02-15 wherein the start command would sometimes exit with: + # + # * Starting database mongodb + # ...fail! + # + # ...not having produced any logs or other output. We couldn't figure out + # what was causing Mongo to fail, so this is a (temporary?) hack to get + # PRs unblocked. - name: start mongod server for tests run: | sudo mkdir -p /data/db sudo chmod -R a+rw /data/db mongod & - - name: set settings path - run: | - echo "settings_path=$(python scripts/unit_test_shards_parser.py --shard-name=${{ matrix.shard_name }} --output settings )" >> $GITHUB_ENV - -# - name: set pytest randomly option -# run: | -# echo "pytest_randomly_option=$(if [ '${{ env.module_name }}' = 'cms' ] || [ '${{ env.module_name }}' = 'common' ]; then echo '-p no:randomly'; else echo '' ; fi)" >> $GITHUB_ENV - - name: install requirements run: | sudo pip install -r requirements/pip.txt sudo pip install -r requirements/edx/testing.txt - if [[ "${{ matrix.django-version }}" == "pinned" ]]; then - sudo pip install -r requirements/edx/django.txt - else - sudo pip install "django~=${{ matrix.django-version }}.0" - fi - - - name: list installed package versions - run: | - sudo pip freeze - - - name: get unit tests for shard - run: | - echo "unit_test_paths=$(python scripts/unit_test_shards_parser.py --shard-name=${{ matrix.shard_name }} )" >> $GITHUB_ENV - - - name: run tests - run: | - python -Wd -m pytest -p no:randomly --ds=${{ env.settings_path }} ${{ env.unit_test_paths }} + sudo pip install "django~=${{ matrix.django-version }}.0" - - name: rename warnings json file - if: success() - run: | - cd test_root/log - mv pytest_warnings.json pytest_warnings_${{ matrix.shard_name }}.json - - - name: save pytest warnings json file - if: success() - uses: actions/upload-artifact@v2 - with: - name: pytest-warnings-json - path: | - test_root/log/pytest_warnings*.json + - name: Setup and run tests + uses: ./.github/actions/unit-tests compile-warnings-report: - runs-on: [ self-hosted ] + runs-on: [ edx-platform-runner ] needs: [ run-tests ] steps: - name: sync directory owner diff --git a/.github/workflows/verify-gha-unit-tests-count.yml b/.github/workflows/verify-gha-unit-tests-count.yml index 7e5950aa94db..70e2295752d8 100644 --- a/.github/workflows/verify-gha-unit-tests-count.yml +++ b/.github/workflows/verify-gha-unit-tests-count.yml @@ -8,6 +8,7 @@ on: jobs: collect-and-verify: + if: github.repository == 'openedx/edx-platform' || github.repository == 'edx/edx-platform-private' runs-on: [ edx-platform-runner ] steps: - name: sync directory owner @@ -19,41 +20,5 @@ jobs: sudo pip install -r requirements/pip.txt sudo pip install -r requirements/edx/testing.txt - - name: collect tests from all modules - run: | - echo "root_cms_unit_tests_count=$(pytest --collect-only --ds=cms.envs.test -p no:warnings cms/ -q | head -n -2 | wc -l)" >> $GITHUB_ENV - echo "root_lms_unit_tests_count=$(pytest --collect-only --ds=lms.envs.test -p no:warnings lms/ openedx/ common/djangoapps/ common/lib/ -q | head -n -2 | wc -l)" >> $GITHUB_ENV - - - name: get GHA unit test paths - run: | - echo "cms_unit_test_paths=$(python scripts/gha_unit_tests_collector.py --cms-only)" >> $GITHUB_ENV - echo "lms_unit_test_paths=$(python scripts/gha_unit_tests_collector.py --lms-only)" >> $GITHUB_ENV - - - - name: collect tests from GHA unit test shards - run: | - echo "cms_unit_tests_count=$(pytest --collect-only --ds=cms.envs.test -p no:warnings ${{ env.cms_unit_test_paths }} -q | head -n -2 | wc -l)" >> $GITHUB_ENV - echo "lms_unit_tests_count=$(pytest --collect-only --ds=lms.envs.test -p no:warnings ${{ env.lms_unit_test_paths }} -q | head -n -2 | wc -l)" >> $GITHUB_ENV - - - - name: add unit tests count - run: | - echo "root_all_unit_tests_count=$((${{ env.root_cms_unit_tests_count }}+${{ env.root_lms_unit_tests_count }}))" >> $GITHUB_ENV - echo "shards_all_unit_tests_count=$((${{ env.cms_unit_tests_count }}+${{ env.lms_unit_tests_count }}))" >> $GITHUB_ENV - - - name: print unit tests count - run: | - echo CMS unit tests from root: ${{ env.root_cms_unit_tests_count }} - echo LMS unit tests from root: ${{ env.root_lms_unit_tests_count }} - echo CMS unit tests from shards: ${{ env.cms_unit_tests_count }} - echo LMS unit tests from shards: ${{ env.lms_unit_tests_count }} - echo All root unit tests count: ${{ env.root_all_unit_tests_count }} - echo All shards unit tests count: ${{ env.shards_all_unit_tests_count }} - - - name: fail the check - if: ${{ env.root_all_unit_tests_count != env.shards_all_unit_tests_count }} - run: | - echo "::error title='Unit test modules in unit-test-shards.json (unit-tests.yml workflow) are outdated'::unit tests running in unit-tests - workflow don't match the count for unit tests for entire edx-platform suite, please update the unit-test-shards.json under .github/workflows - to add any missing apps and match the count. for more details please take a look at scripts/gha-shards-readme.md" - exit 1 + - name: verify unit tests count + uses: ./.github/actions/verify-tests-count From 1a7ea0917eb5678984c4aefb2b6412aac1aa8120 Mon Sep 17 00:00:00 2001 From: Agrendalath <piotr@surowiec.it> Date: Fri, 3 Jun 2022 18:01:44 +0200 Subject: [PATCH 20/20] test: do not collect warnings while counting tests --- .github/actions/verify-tests-count/action.yml | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/.github/actions/verify-tests-count/action.yml b/.github/actions/verify-tests-count/action.yml index 6357a4158c84..80df31b1a174 100644 --- a/.github/actions/verify-tests-count/action.yml +++ b/.github/actions/verify-tests-count/action.yml @@ -6,8 +6,8 @@ runs: - name: collect tests from all modules shell: bash run: | - echo "root_cms_unit_tests_count=$(pytest --collect-only --ds=cms.envs.test cms/ -q | head -n -2 | wc -l)" >> $GITHUB_ENV - echo "root_lms_unit_tests_count=$(pytest --collect-only --ds=lms.envs.test lms/ openedx/ common/djangoapps/ common/lib/ -q | head -n -2 | wc -l)" >> $GITHUB_ENV + echo "root_cms_unit_tests_count=$(pytest --collect-only --ds=cms.envs.test -p no:warnings cms/ -q | head -n -2 | wc -l)" >> $GITHUB_ENV + echo "root_lms_unit_tests_count=$(pytest --collect-only --ds=lms.envs.test -p no:warnings lms/ openedx/ common/djangoapps/ common/lib/ -q | head -n -2 | wc -l)" >> $GITHUB_ENV - name: get GHA unit test paths shell: bash @@ -19,8 +19,8 @@ runs: - name: collect tests from GHA unit test shards shell: bash run: | - echo "cms_unit_tests_count=$(pytest --collect-only --ds=cms.envs.test ${{ env.cms_unit_test_paths }} -q | head -n -2 | wc -l)" >> $GITHUB_ENV - echo "lms_unit_tests_count=$(pytest --collect-only --ds=lms.envs.test ${{ env.lms_unit_test_paths }} -q | head -n -2 | wc -l)" >> $GITHUB_ENV + echo "cms_unit_tests_count=$(pytest --collect-only --ds=cms.envs.test -p no:warnings ${{ env.cms_unit_test_paths }} -q | head -n -2 | wc -l)" >> $GITHUB_ENV + echo "lms_unit_tests_count=$(pytest --collect-only --ds=lms.envs.test -p no:warnings ${{ env.lms_unit_test_paths }} -q | head -n -2 | wc -l)" >> $GITHUB_ENV - name: add unit tests count