Merge branch 'master' into 31696-unused-helper-functions

This commit is contained in:
Edward Zarecor
2023-06-30 12:57:43 +02:00
committed by GitHub
22 changed files with 114 additions and 93 deletions

View File

@@ -119,7 +119,7 @@ RUN nodeenv /edx/app/edxapp/nodeenv --node=16.14.0 --prebuilt
RUN npm install -g npm@8.5.x
COPY package.json package.json
COPY package-lock.json package-lock.json
RUN npm set progress=false && npm install
RUN npm set progress=false && npm ci
# The builder-development stage is a temporary stage that installs python modules required for development purposes
# The built artifacts from this stage are then copied to the development stage.

View File

@@ -125,9 +125,16 @@
<a href="${get_pages_and_resources_url(course_key)}">${_("Pages & Resources")}</a>
</li>
% endif
%if not files_uploads_mfe_enabled:
<li class="nav-item nav-course-courseware-uploads">
<a href="${assets_url}">${_("Files & Uploads")}</a>
</li>
%endif
%if files_uploads_mfe_enabled:
<li class="nav-item nav-course-courseware-uploads">
<a href="${get_files_uploads_url(course_key)}">${_("Files & Media")}</a>
</li>
%endif
% if not pages_and_resources_mfe_enabled:
<li class="nav-item nav-course-courseware-textbooks">
<a href="${textbooks_url}">${_("Textbooks")}</a>
@@ -199,7 +206,7 @@
% endif
% if has_studio_advanced_settings_access(request.user) and advanced_settings_mfe_enabled:
<li class="nav-item nav-course-settings-advanced">
<a href="${get_advanced_settings_url}">${_("Advanced Settings")}</a>
<a href="${get_advanced_settings_url(course_key)}">${_("Advanced Settings")}</a>
</li>
% endif
% if certificates_url:
@@ -222,12 +229,26 @@
<div class="wrapper wrapper-nav-sub">
<div class="nav-sub">
<ul>
% if not import_mfe_enabled:
<li class="nav-item nav-course-tools-import">
<a href="${import_url}">${_("Import")}</a>
</li>
% endif
% if import_mfe_enabled:
<li class="nav-item nav-course-tools-import">
<a href="${get_import_url(course_key)}">${_("Import")}</a>
</li>
% endif
% if not export_mfe_enabled:
<li class="nav-item nav-course-tools-export">
<a href="${export_url}">${_("Export")}</a>
</li>
% endif
% if export_mfe_enabled:
<li class="nav-item nav-course-tools-import">
<a href="${get_export_url(course_key)}">${_("Import")}</a>
</li>
% endif
% if toggles.EXPORT_GIT.is_enabled() and context_course.giturl:
<li class="nav-item nav-course-tools-export-git">
<a href="${reverse('export_git', kwargs=dict(course_key_string=six.text_type(course_key)))}">${_("Export to Git")}</a>

View File

@@ -60,16 +60,18 @@ class AboutPageProductRecommendationsSerializer(serializers.Serializer):
class LearnerDashboardProductRecommendationsSerializer(serializers.Serializer):
"""Serializer for product recommendations for the Learner Dashboard"""
title = serializers.CharField()
courseRunKey = serializers.SerializerMethodField()
marketingUrl = serializers.URLField(source="marketing_url")
courseType = serializers.CharField(source="course_type")
image = CourseImageSerializer()
prospectusPath = serializers.SerializerMethodField()
owners = serializers.ListField(
child=CourseOwnersSerializer(), allow_empty=True
)
courseType = serializers.CharField(source="course_type")
def get_prospectusPath(self, instance):
url_slug = instance.get("url_slug")
return f"course/{url_slug}"
def get_courseRunKey(self, instance):
active_course_run_key = instance.get('active_course_run_key')
return active_course_run_key if active_course_run_key else instance.get('course_runs')[0]['key']
class AboutPageRecommendationsSerializer(serializers.Serializer):

View File

@@ -48,10 +48,12 @@ mock_course_data = [
mock_cross_product_data = [
{
"title": "Title 0",
"courseRunKey": "course-v1:Test+2023_T0",
"marketingUrl": "https://www.marketing_url0.com",
"courseType": "executive-education",
"image": {
"src": "https://www.logo_image_url0.com"
},
"prospectusPath": "course/https://www.marketing_url0.com",
"owners": [
{
"key": "org-0",
@@ -59,14 +61,15 @@ mock_cross_product_data = [
"logoImageUrl": "https://discovery.com/organization/logos/org-0.png"
}
],
"courseType": "executive-education"
},
{
"title": "Title 1",
"courseRunKey": "course-v1:Test+2023_T1",
"marketingUrl": "https://www.marketing_url1.com",
"courseType": "executive-education",
"image": {
"src": "https://www.logo_image_url1.com"
},
"prospectusPath": "course/https://www.marketing_url1.com",
"owners": [
{
"key": "org-1",
@@ -74,7 +77,6 @@ mock_cross_product_data = [
"logoImageUrl": "https://discovery.com/organization/logos/org-1.png"
}
],
"courseType": "executive-education"
},
]
@@ -82,10 +84,12 @@ mock_amplitude_data = [
*mock_cross_product_data,
{
"title": "Title 2",
"courseRunKey": "course-v1:Test+2023_T2",
"marketingUrl": "https://www.marketing_url2.com",
"courseType": "executive-education",
"image": {
"src": "https://www.logo_image_url2.com"
},
"prospectusPath": "course/https://www.marketing_url2.com",
"owners": [
{
"key": "org-2",
@@ -93,14 +97,15 @@ mock_amplitude_data = [
"logoImageUrl": "https://discovery.com/organization/logos/org-2.png"
}
],
"courseType": "executive-education"
},
{
"title": "Title 3",
"courseRunKey": "course-v1:Test+2023_T3",
"marketingUrl": "https://www.marketing_url3.com",
"courseType": "executive-education",
"image": {
"src": "https://www.logo_image_url3.com"
},
"prospectusPath": "course/https://www.marketing_url3.com",
"owners": [
{
"key": "org-3",
@@ -108,7 +113,6 @@ mock_amplitude_data = [
"logoImageUrl": "https://discovery.com/organization/logos/org-3.png"
}
],
"courseType": "executive-education"
}
]
@@ -125,6 +129,11 @@ def get_general_recommendations():
"course_type": "credit-verified-audit",
"logo_image_url": "https://discovery.com/organization/logos/org-1.png",
"marketing_url": "https://www.marketing_url.com",
"course_runs": [
{
"key": "course-v1:MITx+6.00.1x+2T2023",
}
],
"owners": [
{
"key": "MITx",

View File

@@ -97,7 +97,7 @@ class TestCrossProductRecommendationsSerializers(TestCase):
AmplitudeRecommendationsSerializer, and CrossProductAndAmplitudeRecommendations Serializer
"""
def mock_recommended_courses(self, num_of_courses=2, amplitude_courses=False):
def mock_recommended_courses(self, num_of_courses=2):
"""Course data mock"""
recommended_courses = []
@@ -132,20 +132,12 @@ class TestCrossProductRecommendationsSerializers(TestCase):
"marketing_url": f"https://www.marketing_url{index}.com",
"availability": "Current",
},
"active_course_run_key": f"course-v1:Test+2023_T{index}",
"marketing_url": f"https://www.marketing_url{index}.com",
"location_restriction": None
},
)
if amplitude_courses:
keys_to_remove = ["active_course_run", "key", "uuid"]
amplitude_courses = []
for course in recommended_courses:
new_course = {key: value for key, value in course.items() if key not in keys_to_remove}
amplitude_courses.append(new_course)
return amplitude_courses
return recommended_courses
def test_successful_cross_product_recommendation_serialization(self):
@@ -178,7 +170,7 @@ class TestCrossProductRecommendationsSerializers(TestCase):
"""Test that course data serializes correctly for CrossProductAndAmplitudeRecommendationSerializer"""
cross_product_courses = self.mock_recommended_courses(num_of_courses=2)
amplitude_courses = self.mock_recommended_courses(num_of_courses=4, amplitude_courses=True)
amplitude_courses = self.mock_recommended_courses(num_of_courses=4)
serialized_data = CrossProductAndAmplitudeRecommendationsSerializer({
"crossProductCourses": cross_product_courses,

View File

@@ -387,13 +387,12 @@ class TestProductRecommendationsView(APITestCase):
"image": {
"src": "https://www.logo_image_url.com",
},
"url_slug": "https://www.marketing_url.com",
"course_type": "executive-education",
"owners": [
{
"key": "org-1",
"name": "org 1",
"logo_image_url": "https://discovery.com/organization/logos/org-1.png",
"key": "org-1",
"name": "org 1",
"logo_image_url": "https://discovery.com/organization/logos/org-1.png",
},
],
"course_runs": [
@@ -405,6 +404,8 @@ class TestProductRecommendationsView(APITestCase):
"status": "published"
}
],
"marketing_url": "https://www.marketing_url.com/course/some-course",
"advertised_course_run_uuid": f"course-v1:{key}+2023_T2",
}
if keys_with_restriction and key in keys_with_restriction:
course.update({

View File

@@ -213,10 +213,11 @@ class ProductRecommendationsView(APIView):
"title",
"owners",
"image",
"url_slug",
"course_type",
"course_runs",
"location_restriction",
"marketing_url",
"advertised_course_run_uuid",
]
def _get_amplitude_recommendations(self, user, user_country_code):
@@ -262,6 +263,9 @@ class ProductRecommendationsView(APIView):
and course.get("course_runs", [])
and not _has_country_restrictions(course, user_country_code)
):
active_course_run = get_active_course_run(course)
if active_course_run:
course.update({"active_course_run_key": active_course_run.get("key")})
filtered_cross_product_courses.append(course)

View File

@@ -10,23 +10,6 @@ from openedx.core.lib.cache_utils import request_cached
from .models import BlockStructureConfiguration
# Switches
# .. toggle_name: block_structure.invalidate_cache_on_publish
# .. toggle_implementation: WaffleSwitch
# .. toggle_default: False
# .. toggle_description: When enabled, the block structure cache is invalidated when changes to
# courses are published. If `block_structure.storage_backing_for_cache` is active, all block
# structures related to the published course are also cleared from storage.
# .. toggle_warning: This switch will likely be deprecated and removed.
# .. toggle_use_cases: temporary
# .. toggle_creation_date: 2017-02-23
# .. toggle_target_removal_date: 2017-05-23
# .. toggle_tickets: https://github.com/openedx/edx-platform/pull/14358,
# https://github.com/openedx/edx-platform/pull/14571,
# https://openedx.atlassian.net/browse/DEPR-144
INVALIDATE_CACHE_ON_PUBLISH = WaffleSwitch(
"block_structure.invalidate_cache_on_publish", __name__
)
# .. toggle_name: block_structure.storage_backing_for_cache
# .. toggle_implementation: WaffleSwitch
# .. toggle_default: False

View File

@@ -10,9 +10,7 @@ from opaque_keys.edx.locator import LibraryLocator
from xmodule.modulestore.django import SignalHandler
from . import config
from .api import clear_course_from_cache
from .models import BlockStructureNotFound
from .tasks import update_course_in_cache_v2
log = logging.getLogger(__name__)
@@ -28,15 +26,6 @@ def update_block_structure_on_course_publish(sender, course_key, **kwargs): # p
if isinstance(course_key, LibraryLocator):
return
if config.INVALIDATE_CACHE_ON_PUBLISH.is_enabled():
try:
clear_course_from_cache(course_key)
except BlockStructureNotFound:
log.warning(
"BlockStructure: %s not found when trying to clear course from cache",
course_key,
)
update_course_in_cache_v2.apply_async(
kwargs=dict(course_id=str(course_key)),
countdown=settings.BLOCK_STRUCTURES_SETTINGS['COURSE_PUBLISH_TASK_DELAY'],

View File

@@ -5,14 +5,12 @@ from unittest.mock import patch
import pytest
import ddt
from edx_toggles.toggles.testutils import override_waffle_switch
from opaque_keys.edx.locator import CourseLocator, LibraryLocator
from xmodule.modulestore.exceptions import ItemNotFoundError
from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
from xmodule.modulestore.tests.factories import CourseFactory
from ..api import get_block_structure_manager
from ..config import INVALIDATE_CACHE_ON_PUBLISH
from ..signals import update_block_structure_on_course_publish
from .helpers import is_course_in_block_structure_cache
@@ -45,17 +43,6 @@ class CourseBlocksSignalTest(ModuleStoreTestCase):
updated_block_structure = bs_manager.get_collected()
assert test_display_name == updated_block_structure.get_xblock_field(self.course_usage_key, 'display_name')
@ddt.data(True, False)
@patch('openedx.core.djangoapps.content.block_structure.manager.BlockStructureManager.clear')
def test_cache_invalidation(self, invalidate_cache_enabled, mock_bs_manager_clear):
test_display_name = "Jedi 101"
with override_waffle_switch(INVALIDATE_CACHE_ON_PUBLISH, active=invalidate_cache_enabled):
self.course.display_name = test_display_name
self.update_course(self.course, self.user.id)
assert mock_bs_manager_clear.called == invalidate_cache_enabled
def test_course_delete(self):
bs_manager = get_block_structure_manager(self.course.id)
assert bs_manager.get_collected() is not None

View File

@@ -0,0 +1,23 @@
# Generated by Django 3.2.19 on 2023-06-28 13:35
from django.db import migrations, models
class Migration(migrations.Migration):
dependencies = [
('course_overviews', '0027_auto_20221102_1109'),
]
operations = [
migrations.AddField(
model_name='courseoverview',
name='force_on_flexible_peer_openassessments',
field=models.BooleanField(default=False),
),
migrations.AddField(
model_name='historicalcourseoverview',
name='force_on_flexible_peer_openassessments',
field=models.BooleanField(default=False),
),
]

View File

@@ -64,7 +64,7 @@ class CourseOverview(TimeStampedModel):
app_label = 'course_overviews'
# IMPORTANT: Bump this whenever you modify this model and/or add a migration.
VERSION = 18
VERSION = 19
# Cache entry versioning.
version = models.IntegerField()
@@ -144,6 +144,9 @@ class CourseOverview(TimeStampedModel):
entrance_exam_id = models.CharField(max_length=255, blank=True)
entrance_exam_minimum_score_pct = models.FloatField(default=0.65)
# Open Response Assessment configuration
force_on_flexible_peer_openassessments = models.BooleanField(default=False)
external_id = models.CharField(max_length=128, null=True, blank=True)
language = models.TextField(null=True)
@@ -268,6 +271,8 @@ class CourseOverview(TimeStampedModel):
else:
course_overview.entrance_exam_minimum_score_pct = course.entrance_exam_minimum_score_pct
course_overview.force_on_flexible_peer_openassessments = course.force_on_flexible_peer_openassessments
if not CatalogIntegration.is_enabled():
course_overview.language = course.language

View File

@@ -87,7 +87,7 @@ class TestPaverNodeInstall(PaverTestCase):
def test_npm_install_with_subprocess_error(self):
"""
Test an error in 'npm install' execution
Test an error in 'npm ci' execution
"""
with patch('subprocess.Popen') as _mock_popen:
_mock_subprocess = mock.Mock()
@@ -97,21 +97,21 @@ class TestPaverNodeInstall(PaverTestCase):
with pytest.raises(Exception):
pavelib.prereqs.node_prereqs_installation()
# npm install will be called twice
# npm ci will be called twice
assert _mock_popen.call_count == 2
def test_npm_install_called_once_when_successful(self):
"""
Vanilla npm install should only be calling npm install one time
Vanilla npm ci should only be calling npm ci one time
"""
with patch('subprocess.Popen') as _mock_popen:
pavelib.prereqs.node_prereqs_installation()
# when there's no failure, npm install is only called once
# when there's no failure, npm ci is only called once
assert _mock_popen.call_count == 1
def test_npm_install_with_unexpected_subprocess_error(self):
"""
If there's some other error, only call npm install once, and raise a failure
If there's some other error, only call npm ci once, and raise a failure
"""
with patch('subprocess.Popen') as _mock_popen:
_mock_popen.side_effect = unexpected_fail_on_npm_install

View File

@@ -137,7 +137,7 @@ def node_prereqs_installation():
else:
npm_log_file_path = f'{Env.GEN_LOG_DIR}/npm-install.log'
npm_log_file = open(npm_log_file_path, 'wb') # lint-amnesty, pylint: disable=consider-using-with
npm_command = 'npm clean-install --verbose'.split()
npm_command = 'npm ci --verbose'.split()
# The implementation of Paver's `sh` function returns before the forked
# actually returns. Using a Popen object so that we can ensure that
@@ -145,14 +145,7 @@ def node_prereqs_installation():
proc = subprocess.Popen(npm_command, stderr=npm_log_file) # lint-amnesty, pylint: disable=consider-using-with
retcode = proc.wait()
if retcode == 1:
# Error handling around a race condition that produces "cb() never called" error. This
# evinces itself as `cb_error_text` and it ought to disappear when we upgrade
# npm to 3 or higher. TODO: clean this up when we do that.
print("npm clean-install error detected. Retrying...")
proc = subprocess.Popen(npm_command, stderr=npm_log_file) # lint-amnesty, pylint: disable=consider-using-with
retcode = proc.wait()
if retcode == 1:
raise Exception(f"npm install failed: See {npm_log_file_path}")
raise Exception(f"npm install failed: See {npm_log_file_path}")
print("Successfully clean-installed NPM packages. Log found at {}".format(
npm_log_file_path
))

View File

@@ -32,7 +32,7 @@ django-storages==1.9.1
# The team that owns this package will manually bump this package rather than having it pulled in automatically.
# This is to allow them to better control its deployment and to do it in a process that works better
# for them.
edx-enterprise==3.67.5
edx-enterprise==3.67.7
# oauthlib>3.0.1 causes test failures ( also remove the django-oauth-toolkit constraint when this is fixed )
oauthlib==3.0.1

View File

@@ -489,7 +489,7 @@ edx-drf-extensions==8.8.0
# edx-when
# edxval
# learner-pathway-progress
edx-enterprise==3.67.5
edx-enterprise==3.67.7
# via
# -c requirements/edx/../constraints.txt
# -r requirements/edx/base.in

View File

@@ -617,7 +617,7 @@ edx-drf-extensions==8.8.0
# edx-when
# edxval
# learner-pathway-progress
edx-enterprise==3.67.5
edx-enterprise==3.67.7
# via
# -c requirements/edx/../constraints.txt
# -r requirements/edx/testing.txt

View File

@@ -590,7 +590,7 @@ edx-drf-extensions==8.8.0
# edx-when
# edxval
# learner-pathway-progress
edx-enterprise==3.67.5
edx-enterprise==3.67.7
# via
# -c requirements/edx/../constraints.txt
# -r requirements/edx/base.txt

View File

@@ -115,6 +115,11 @@ ignore_imports =
# cms.djangoapps.export_course_metadata.tasks
# -> openedx.core.djangoapps.schedules.content_highlights
# -> lms.djangoapps.courseware.block_render & lms.djangoapps.courseware.model_data
openedx.core.djangoapps.content_libraries.* -> lms.djangoapps.grades.api
# cms.djangoapps.contentstore.tasks -> openedx.core.djangoapps.content_libraries.models
# -> openedx.core.djangoapps.content_libraries.apps
# -> openedx.core.djangoapps.content_libraries.signal_handlers
# -> lms.djangoapps.grades.api
openedx.core.djangoapps.schedules.content_highlights -> lms.djangoapps.courseware.*
# cms.djangoapps.contentstore.[various]
# -> openedx.core.lib.gating.api

View File

@@ -978,6 +978,13 @@ class CourseFields: # lint-amnesty, pylint: disable=missing-class-docstring
]
)
force_on_flexible_peer_openassessments = Boolean(
display_name=_("Force Flexible Grading for Peer ORAs"),
help=_("Setting this flag will force on the flexible grading option for all peer-graded ORAs in this course."),
scope=Scope.settings,
default=False,
)
"""
instructor_info dict structure:
{

View File

@@ -879,7 +879,7 @@ div.problem {
// ====================
.problem {
.inputtype.option-input {
margin: (-$baseline/2) 0 $baseline;
margin: 0 0 0 0 !important;
.indicator-container {
display: inline-block;