From aa70fea890ecd622d392270cd21453b9ae9fe578 Mon Sep 17 00:00:00 2001 From: Asespinel <79876430+Asespinel@users.noreply.github.com> Date: Wed, 19 Jun 2024 09:22:25 -0500 Subject: [PATCH 01/18] feat: hide the survey report banner for a month after clicking the dismiss button (#34914) This hides the survey report banner from the Django Admin for a particular user for one month after they click on the "dismiss" button. This is done completely on the client side using localStorage, so the same user could see the banner again if they're logging in with a different browser. --- .../static/survey_report/js/admin_banner.js | 46 +++++++++++++++++-- .../templates/survey_report/admin_banner.html | 3 +- 2 files changed, 45 insertions(+), 4 deletions(-) diff --git a/openedx/features/survey_report/static/survey_report/js/admin_banner.js b/openedx/features/survey_report/static/survey_report/js/admin_banner.js index d8650321ba36..8dd9ba5565e0 100644 --- a/openedx/features/survey_report/static/survey_report/js/admin_banner.js +++ b/openedx/features/survey_report/static/survey_report/js/admin_banner.js @@ -1,11 +1,51 @@ $(document).ready(function(){ + // Function to get user ID + function getUserId() { + return $('#userIdSurvey').val(); + } + + // Function to get current time in milliseconds + function getCurrentTime() { + return new Date().getTime(); + } + + // Function to set dismissal time and expiration time in local storage + function setDismissalAndExpirationTime(userId, dismissalTime) { + let expirationTime = dismissalTime + (30 * 24 * 60 * 60 * 1000); // 30 days + localStorage.setItem('bannerDismissalTime_' + userId, dismissalTime); + localStorage.setItem('bannerExpirationTime_' + userId, expirationTime); + } + + // Function to check if banner should be shown or hidden + function checkBannerVisibility() { + let userId = getUserId(); + let bannerDismissalTime = localStorage.getItem('bannerDismissalTime_' + userId); + let bannerExpirationTime = localStorage.getItem('bannerExpirationTime_' + userId); + let currentTime = getCurrentTime(); + + if (bannerDismissalTime && bannerExpirationTime && currentTime > bannerExpirationTime) { + // Banner was dismissed and it's not within the expiration period, so show it + $('#originalContent').show(); + } else if (bannerDismissalTime && bannerExpirationTime && currentTime < bannerExpirationTime) { + // Banner was dismissed and it's within the expiration period, so hide it + $('#originalContent').hide(); + } else { + // Banner has not been dismissed ever so we need to show it. + $('#originalContent').show(); + } + } + + // Click event for dismiss button $('#dismissButton').click(function() { $('#originalContent').slideUp('slow', function() { - // If you want to do something after the slide-up, do it here. - // For example, you can hide the entire div: - // $(this).hide(); + let userId = getUserId(); + let dismissalTime = getCurrentTime(); + setDismissalAndExpirationTime(userId, dismissalTime); }); }); + + // Check banner visibility on page load + checkBannerVisibility(); // When the form is submitted $("#survey_report_form").submit(function(event){ event.preventDefault(); // Prevent the form from submitting traditionally diff --git a/openedx/features/survey_report/templates/survey_report/admin_banner.html b/openedx/features/survey_report/templates/survey_report/admin_banner.html index fa0b37ecf751..e13eb655f63c 100644 --- a/openedx/features/survey_report/templates/survey_report/admin_banner.html +++ b/openedx/features/survey_report/templates/survey_report/admin_banner.html @@ -1,7 +1,7 @@ {% block survey_report_banner %} {% load static %} {% if show_survey_report_banner %} -
+ From e5f074b45d94741a6476b3e471d2bf9042be7efe Mon Sep 17 00:00:00 2001 From: magajh Date: Mon, 15 Jul 2024 09:23:25 -0400 Subject: [PATCH 02/18] chore: upgrade Django to 4.2.14 --- requirements/edx/base.txt | 2 +- requirements/edx/development.txt | 2 +- requirements/edx/doc.txt | 2 +- requirements/edx/testing.txt | 2 +- scripts/user_retirement/requirements/base.txt | 2 +- scripts/user_retirement/requirements/testing.txt | 2 +- 6 files changed, 6 insertions(+), 6 deletions(-) diff --git a/requirements/edx/base.txt b/requirements/edx/base.txt index 2ae5ba235a89..db14ceee890f 100644 --- a/requirements/edx/base.txt +++ b/requirements/edx/base.txt @@ -177,7 +177,7 @@ defusedxml==0.7.1 # ora2 # python3-openid # social-auth-core -django==4.2.13 +django==4.2.14 # via # -c requirements/edx/../common_constraints.txt # -c requirements/edx/../constraints.txt diff --git a/requirements/edx/development.txt b/requirements/edx/development.txt index 8b1bb858efe2..cda3abc1db76 100644 --- a/requirements/edx/development.txt +++ b/requirements/edx/development.txt @@ -347,7 +347,7 @@ distlib==0.3.8 # via # -r requirements/edx/testing.txt # virtualenv -django==4.2.13 +django==4.2.14 # via # -c requirements/edx/../common_constraints.txt # -c requirements/edx/../constraints.txt diff --git a/requirements/edx/doc.txt b/requirements/edx/doc.txt index 9e1dee112300..d766c8f0298a 100644 --- a/requirements/edx/doc.txt +++ b/requirements/edx/doc.txt @@ -227,7 +227,7 @@ defusedxml==0.7.1 # ora2 # python3-openid # social-auth-core -django==4.2.13 +django==4.2.14 # via # -c requirements/edx/../common_constraints.txt # -c requirements/edx/../constraints.txt diff --git a/requirements/edx/testing.txt b/requirements/edx/testing.txt index 452fec9a8643..67cdbe56435b 100644 --- a/requirements/edx/testing.txt +++ b/requirements/edx/testing.txt @@ -263,7 +263,7 @@ dill==0.3.8 # via pylint distlib==0.3.8 # via virtualenv -django==4.2.13 +django==4.2.14 # via # -c requirements/edx/../common_constraints.txt # -c requirements/edx/../constraints.txt diff --git a/scripts/user_retirement/requirements/base.txt b/scripts/user_retirement/requirements/base.txt index 934f4293d699..1e510e645926 100644 --- a/scripts/user_retirement/requirements/base.txt +++ b/scripts/user_retirement/requirements/base.txt @@ -39,7 +39,7 @@ click==8.1.6 # edx-django-utils cryptography==42.0.7 # via pyjwt -django==4.2.13 +django==4.2.14 # via # -c scripts/user_retirement/requirements/../../../requirements/common_constraints.txt # -c scripts/user_retirement/requirements/../../../requirements/constraints.txt diff --git a/scripts/user_retirement/requirements/testing.txt b/scripts/user_retirement/requirements/testing.txt index 2bae9bb1a272..15cc2d9849bf 100644 --- a/scripts/user_retirement/requirements/testing.txt +++ b/scripts/user_retirement/requirements/testing.txt @@ -56,7 +56,7 @@ cryptography==42.0.7 # pyjwt ddt==1.7.2 # via -r scripts/user_retirement/requirements/testing.in -django==4.2.13 +django==4.2.14 # via # -r scripts/user_retirement/requirements/base.txt # django-crum From 4744ea1c39f22d397a0206a4761d833496a2851d Mon Sep 17 00:00:00 2001 From: Muhammad Soban Javed Date: Mon, 10 Jun 2024 23:31:34 +0500 Subject: [PATCH 03/18] chore!: uprgade social-auth-app-django to version 5.4.1 --- requirements/constraints.txt | 8 -------- requirements/edx/base.txt | 7 +++---- requirements/edx/development.txt | 7 +++---- requirements/edx/doc.txt | 7 +++---- requirements/edx/testing.txt | 7 +++---- 5 files changed, 12 insertions(+), 24 deletions(-) diff --git a/requirements/constraints.txt b/requirements/constraints.txt index 8f7a66f9cc2f..866ac9e3bf7d 100644 --- a/requirements/constraints.txt +++ b/requirements/constraints.txt @@ -62,14 +62,6 @@ pycodestyle<2.9.0 pylint<2.16.0 # greater version failing quality test. Fix them in seperate ticket. -# adding these constraints to minimize boto3 and botocore changeset -social-auth-core==4.3.0 - -# social-auth-app-django versions after 5.2.0 has a problematic migration that will cause issues deployments with large -# `social_auth_usersocialauth` tables. 5.1.0 has missing migration and 5.2.0 has that problematic migration. -social-auth-app-django==5.0.0 - - # urllib3>=2.0.0 conflicts with elastic search && snowflake-connector-python packages # which require urllib3<2 for now. # Issue for unpinning: https://github.com/openedx/edx-platform/issues/32222 diff --git a/requirements/edx/base.txt b/requirements/edx/base.txt index db14ceee890f..44fbf21df2eb 100644 --- a/requirements/edx/base.txt +++ b/requirements/edx/base.txt @@ -246,6 +246,7 @@ django==4.2.14 # openedx-filters # openedx-learning # ora2 + # social-auth-app-django # super-csv # xblock-google-drive # xss-utils @@ -1097,14 +1098,12 @@ slumber==0.7.1 # edx-rest-api-client snowflake-connector-python==3.10.0 # via edx-enterprise -social-auth-app-django==5.0.0 +social-auth-app-django==5.4.1 # via - # -c requirements/edx/../constraints.txt # -r requirements/edx/kernel.in # edx-auth-backends -social-auth-core==4.3.0 +social-auth-core==4.5.4 # via - # -c requirements/edx/../constraints.txt # -r requirements/edx/kernel.in # edx-auth-backends # social-auth-app-django diff --git a/requirements/edx/development.txt b/requirements/edx/development.txt index cda3abc1db76..93e4b117a852 100644 --- a/requirements/edx/development.txt +++ b/requirements/edx/development.txt @@ -420,6 +420,7 @@ django==4.2.14 # openedx-filters # openedx-learning # ora2 + # social-auth-app-django # super-csv # xblock-google-drive # xss-utils @@ -1945,15 +1946,13 @@ snowflake-connector-python==3.10.0 # -r requirements/edx/doc.txt # -r requirements/edx/testing.txt # edx-enterprise -social-auth-app-django==5.0.0 +social-auth-app-django==5.4.1 # via - # -c requirements/edx/../constraints.txt # -r requirements/edx/doc.txt # -r requirements/edx/testing.txt # edx-auth-backends -social-auth-core==4.3.0 +social-auth-core==4.5.4 # via - # -c requirements/edx/../constraints.txt # -r requirements/edx/doc.txt # -r requirements/edx/testing.txt # edx-auth-backends diff --git a/requirements/edx/doc.txt b/requirements/edx/doc.txt index d766c8f0298a..d5d3136349f1 100644 --- a/requirements/edx/doc.txt +++ b/requirements/edx/doc.txt @@ -296,6 +296,7 @@ django==4.2.14 # openedx-filters # openedx-learning # ora2 + # social-auth-app-django # super-csv # xblock-google-drive # xss-utils @@ -1300,14 +1301,12 @@ snowflake-connector-python==3.10.0 # via # -r requirements/edx/base.txt # edx-enterprise -social-auth-app-django==5.0.0 +social-auth-app-django==5.4.1 # via - # -c requirements/edx/../constraints.txt # -r requirements/edx/base.txt # edx-auth-backends -social-auth-core==4.3.0 +social-auth-core==4.5.4 # via - # -c requirements/edx/../constraints.txt # -r requirements/edx/base.txt # edx-auth-backends # social-auth-app-django diff --git a/requirements/edx/testing.txt b/requirements/edx/testing.txt index 67cdbe56435b..3c109ddaf98f 100644 --- a/requirements/edx/testing.txt +++ b/requirements/edx/testing.txt @@ -332,6 +332,7 @@ django==4.2.14 # openedx-filters # openedx-learning # ora2 + # social-auth-app-django # super-csv # xblock-google-drive # xss-utils @@ -1466,14 +1467,12 @@ snowflake-connector-python==3.10.0 # via # -r requirements/edx/base.txt # edx-enterprise -social-auth-app-django==5.0.0 +social-auth-app-django==5.4.1 # via - # -c requirements/edx/../constraints.txt # -r requirements/edx/base.txt # edx-auth-backends -social-auth-core==4.3.0 +social-auth-core==4.5.4 # via - # -c requirements/edx/../constraints.txt # -r requirements/edx/base.txt # edx-auth-backends # social-auth-app-django From 670772b2b9a677560775f9a5b4959de00f7571ab Mon Sep 17 00:00:00 2001 From: Muhammad Soban Javed Date: Fri, 28 Jun 2024 01:49:59 +0500 Subject: [PATCH 04/18] chore: add migration from social_django --- ...ricalusersocialauth_extra_data_and_more.py | 23 +++++++++++++++++++ 1 file changed, 23 insertions(+) create mode 100644 lms/djangoapps/support/migrations/0006_alter_historicalusersocialauth_extra_data_and_more.py diff --git a/lms/djangoapps/support/migrations/0006_alter_historicalusersocialauth_extra_data_and_more.py b/lms/djangoapps/support/migrations/0006_alter_historicalusersocialauth_extra_data_and_more.py new file mode 100644 index 000000000000..5f09d0cc493b --- /dev/null +++ b/lms/djangoapps/support/migrations/0006_alter_historicalusersocialauth_extra_data_and_more.py @@ -0,0 +1,23 @@ +# Generated by Django 4.2.13 on 2024-06-27 20:46 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('support', '0005_unique_course_id'), + ] + + operations = [ + migrations.AlterField( + model_name='historicalusersocialauth', + name='extra_data', + field=models.JSONField(default=dict), + ), + migrations.AlterField( + model_name='historicalusersocialauth', + name='id', + field=models.BigIntegerField(auto_created=True, blank=True, db_index=True, verbose_name='ID'), + ), + ] From 2165da0a344db6f1e18af530aa6c699135570793 Mon Sep 17 00:00:00 2001 From: Feanil Patel Date: Tue, 23 Jul 2024 10:51:32 -0400 Subject: [PATCH 05/18] chore: Re-compile requirements. --- requirements/common_constraints.txt | 8 ++++++++ requirements/edx/base.txt | 1 + requirements/edx/development.txt | 1 + requirements/edx/doc.txt | 1 + requirements/edx/testing.txt | 1 + 5 files changed, 12 insertions(+) diff --git a/requirements/common_constraints.txt b/requirements/common_constraints.txt index 4abc9ae22cb3..ef8bc86061b7 100644 --- a/requirements/common_constraints.txt +++ b/requirements/common_constraints.txt @@ -21,6 +21,7 @@ Django<5.0 # elasticsearch>=7.14.0 includes breaking changes in it which caused issues in discovery upgrade process. # elastic search changelog: https://www.elastic.co/guide/en/enterprise-search/master/release-notes-7.14.0.html +# See https://github.com/openedx/edx-platform/issues/35126 for more info elasticsearch<7.14.0 # django-simple-history>3.0.0 adds indexing and causes a lot of migrations to be affected @@ -33,3 +34,10 @@ elasticsearch<7.14.0 # So we need to pin it globally, for now. # Ticket for unpinning: https://github.com/openedx/edx-lint/issues/407 importlib-metadata<7 + +# Cause: https://github.com/openedx/event-tracking/pull/290 +# event-tracking 2.4.1 upgrades to pymongo 4.4.0 which is not supported on edx-platform. +# We will pin event-tracking to do not break existing installations +# This can be unpinned once https://github.com/openedx/edx-platform/issues/34586 +# has been resolved and edx-platform is running with pymongo>=4.4.0 +event-tracking<2.4.1 diff --git a/requirements/edx/base.txt b/requirements/edx/base.txt index 44fbf21df2eb..d68c66cc6ef4 100644 --- a/requirements/edx/base.txt +++ b/requirements/edx/base.txt @@ -567,6 +567,7 @@ enmerkar-underscore==2.3.0 # via -r requirements/edx/kernel.in event-tracking==2.4.0 # via + # -c requirements/edx/../common_constraints.txt # -r requirements/edx/kernel.in # edx-completion # edx-proctoring diff --git a/requirements/edx/development.txt b/requirements/edx/development.txt index 93e4b117a852..f78a27d03813 100644 --- a/requirements/edx/development.txt +++ b/requirements/edx/development.txt @@ -895,6 +895,7 @@ enmerkar-underscore==2.3.0 # -r requirements/edx/testing.txt event-tracking==2.4.0 # via + # -c requirements/edx/../common_constraints.txt # -r requirements/edx/doc.txt # -r requirements/edx/testing.txt # edx-completion diff --git a/requirements/edx/doc.txt b/requirements/edx/doc.txt index d5d3136349f1..de65c64de287 100644 --- a/requirements/edx/doc.txt +++ b/requirements/edx/doc.txt @@ -651,6 +651,7 @@ enmerkar-underscore==2.3.0 # via -r requirements/edx/base.txt event-tracking==2.4.0 # via + # -c requirements/edx/../common_constraints.txt # -r requirements/edx/base.txt # edx-completion # edx-proctoring diff --git a/requirements/edx/testing.txt b/requirements/edx/testing.txt index 3c109ddaf98f..0baf5e170300 100644 --- a/requirements/edx/testing.txt +++ b/requirements/edx/testing.txt @@ -688,6 +688,7 @@ enmerkar-underscore==2.3.0 # via -r requirements/edx/base.txt event-tracking==2.4.0 # via + # -c requirements/edx/../common_constraints.txt # -r requirements/edx/base.txt # edx-completion # edx-proctoring From ed72248eb6acc785acc989148bf8f12d518d296e Mon Sep 17 00:00:00 2001 From: connorhaugh Date: Thu, 15 Feb 2024 18:04:49 +0000 Subject: [PATCH 06/18] fix: libraries across orgs --- cms/djangoapps/contentstore/utils.py | 4 +- cms/djangoapps/contentstore/views/library.py | 53 +++++++++++++++---- .../contentstore/views/tests/test_library.py | 39 +++++++++----- 3 files changed, 70 insertions(+), 26 deletions(-) diff --git a/cms/djangoapps/contentstore/utils.py b/cms/djangoapps/contentstore/utils.py index 9bcd77c3eea9..1b5af4f769bd 100644 --- a/cms/djangoapps/contentstore/utils.py +++ b/cms/djangoapps/contentstore/utils.py @@ -1670,7 +1670,7 @@ def get_home_context(request, no_course=False): LIBRARY_AUTHORING_MICROFRONTEND_URL, LIBRARIES_ENABLED, should_redirect_to_library_authoring_mfe, - user_can_create_library, + user_can_view_create_library_button, ) active_courses = [] @@ -1699,7 +1699,7 @@ def get_home_context(request, no_course=False): 'library_authoring_mfe_url': LIBRARY_AUTHORING_MICROFRONTEND_URL, 'taxonomy_list_mfe_url': get_taxonomy_list_url(), 'libraries': libraries, - 'show_new_library_button': user_can_create_library(user) and not should_redirect_to_library_authoring_mfe(), + 'show_new_library_button': user_can_view_create_library_button(user) and not should_redirect_to_library_authoring_mfe(), 'user': user, 'request_course_creator_url': reverse('request_course_creator'), 'course_creator_status': _get_course_creator_status(user), diff --git a/cms/djangoapps/contentstore/views/library.py b/cms/djangoapps/contentstore/views/library.py index 17aa24c5712a..8ea6bf1e3657 100644 --- a/cms/djangoapps/contentstore/views/library.py +++ b/cms/djangoapps/contentstore/views/library.py @@ -10,7 +10,7 @@ from django.conf import settings from django.contrib.auth.decorators import login_required from django.core.exceptions import PermissionDenied -from django.http import Http404, HttpResponseForbidden, HttpResponseNotAllowed +from django.http import Http404, HttpResponseNotAllowed from django.utils.translation import gettext as _ from django.views.decorators.csrf import ensure_csrf_cookie from django.views.decorators.http import require_http_methods @@ -68,13 +68,10 @@ def should_redirect_to_library_authoring_mfe(): REDIRECT_TO_LIBRARY_AUTHORING_MICROFRONTEND.is_enabled() ) - -def user_can_create_library(user, org=None): +def user_can_view_create_library_button(user): """ - Helper method for returning the library creation status for a particular user, - taking into account the value LIBRARIES_ENABLED. + Helper method for displaying the visibilty of the create_library_button. """ - if not LIBRARIES_ENABLED: return False elif user.is_staff: @@ -84,8 +81,48 @@ def user_can_create_library(user, org=None): has_org_staff_role = OrgStaffRole().get_orgs_for_user(user).exists() has_course_staff_role = UserBasedRole(user=user, role=CourseStaffRole.ROLE).courses_with_role().exists() has_course_admin_role = UserBasedRole(user=user, role=CourseInstructorRole.ROLE).courses_with_role().exists() + return is_course_creator or has_org_staff_role or has_course_staff_role or has_course_admin_role + else: + # EDUCATOR-1924: DISABLE_LIBRARY_CREATION overrides DISABLE_COURSE_CREATION, if present. + disable_library_creation = settings.FEATURES.get('DISABLE_LIBRARY_CREATION', None) + disable_course_creation = settings.FEATURES.get('DISABLE_COURSE_CREATION', False) + if disable_library_creation is not None: + return not disable_library_creation + else: + return not disable_course_creation + + +def user_can_create_library(user, org): + """ + Helper method for returning the library creation status for a particular user, + taking into account the value LIBRARIES_ENABLED. + + users can only create libraries in orgs they are a part of. + """ + if org is None: + return False + if not LIBRARIES_ENABLED: + return False + elif user.is_staff: + return True + if settings.FEATURES.get('ENABLE_CREATOR_GROUP', False): + is_course_creator = get_course_creator_status(user) == 'granted' + has_org_staff_role = org in OrgStaffRole().get_orgs_for_user(user) + has_course_staff_role = ( + UserBasedRole(user=user, role=CourseStaffRole.ROLE) + .courses_with_role() + .filter(org=org) + .exists() + ) + has_course_admin_role = ( + UserBasedRole(user=user, role=CourseInstructorRole.ROLE) + .courses_with_role() + .filter(org=org) + .exists() + ) return is_course_creator or has_org_staff_role or has_course_staff_role or has_course_admin_role + else: # EDUCATOR-1924: DISABLE_LIBRARY_CREATION overrides DISABLE_COURSE_CREATION, if present. disable_library_creation = settings.FEATURES.get('DISABLE_LIBRARY_CREATION', None) @@ -108,12 +145,8 @@ def library_handler(request, library_key_string=None): raise Http404 # Should never happen because we test the feature in urls.py also if request.method == 'POST': - if not user_can_create_library(request.user): - return HttpResponseForbidden() - if library_key_string is not None: return HttpResponseNotAllowed(("POST",)) - return _create_library(request) else: diff --git a/cms/djangoapps/contentstore/views/tests/test_library.py b/cms/djangoapps/contentstore/views/tests/test_library.py index f6b7a48a68e1..fdbb9d905c62 100644 --- a/cms/djangoapps/contentstore/views/tests/test_library.py +++ b/cms/djangoapps/contentstore/views/tests/test_library.py @@ -59,55 +59,66 @@ def setUp(self): @mock.patch("cms.djangoapps.contentstore.views.library.LIBRARIES_ENABLED", False) def test_library_creator_status_libraries_not_enabled(self): _, nostaff_user = self.create_non_staff_authed_user_client() - self.assertEqual(user_can_create_library(nostaff_user), False) + self.assertEqual(user_can_create_library(nostaff_user, None), False) # When creator group is disabled, non-staff users can create libraries @mock.patch("cms.djangoapps.contentstore.views.library.LIBRARIES_ENABLED", True) def test_library_creator_status_with_no_course_creator_role(self): _, nostaff_user = self.create_non_staff_authed_user_client() - self.assertEqual(user_can_create_library(nostaff_user), True) + self.assertEqual(user_can_create_library(nostaff_user, 'An Org'), True) # When creator group is enabled, Non staff users cannot create libraries @mock.patch("cms.djangoapps.contentstore.views.library.LIBRARIES_ENABLED", True) def test_library_creator_status_for_enabled_creator_group_setting_for_non_staff_users(self): _, nostaff_user = self.create_non_staff_authed_user_client() with mock.patch.dict('django.conf.settings.FEATURES', {"ENABLE_CREATOR_GROUP": True}): - self.assertEqual(user_can_create_library(nostaff_user), False) + self.assertEqual(user_can_create_library(nostaff_user, None), False) - # Global staff can create libraries + # Global staff can create libraries for any org, even ones that don't exist. @mock.patch("cms.djangoapps.contentstore.views.library.LIBRARIES_ENABLED", True) def test_library_creator_status_with_is_staff_user(self): - self.assertEqual(user_can_create_library(self.user), True) + print(self.user.is_staff) + self.assertEqual(user_can_create_library(self.user, 'aNyOrg'), True) - # When creator groups are enabled, global staff can create libraries + # Global staff can create libraries for any org, but an org has to be supplied. + @mock.patch("cms.djangoapps.contentstore.views.library.LIBRARIES_ENABLED", True) + def test_library_creator_status_with_is_staff_user_no_org(self): + print(self.user.is_staff) + self.assertEqual(user_can_create_library(self.user, None), False) + + # When creator groups are enabled, global staff can create libraries in any org @mock.patch("cms.djangoapps.contentstore.views.library.LIBRARIES_ENABLED", True) def test_library_creator_status_for_enabled_creator_group_setting_with_is_staff_user(self): with mock.patch.dict('django.conf.settings.FEATURES', {"ENABLE_CREATOR_GROUP": True}): - self.assertEqual(user_can_create_library(self.user), True) + self.assertEqual(user_can_create_library(self.user, 'RandomOrg'), True) - # When creator groups are enabled, course creators can create libraries + # When creator groups are enabled, course creators can create libraries in any org. @mock.patch("cms.djangoapps.contentstore.views.library.LIBRARIES_ENABLED", True) def test_library_creator_status_with_course_creator_role_for_enabled_creator_group_setting(self): _, nostaff_user = self.create_non_staff_authed_user_client() with mock.patch.dict('django.conf.settings.FEATURES', {"ENABLE_CREATOR_GROUP": True}): grant_course_creator_status(self.user, nostaff_user) - self.assertEqual(user_can_create_library(nostaff_user), True) + self.assertEqual(user_can_create_library(nostaff_user, 'soMeRandOmoRg'), True) # When creator groups are enabled, course staff members can create libraries + # but only in the org they are course staff for. @mock.patch("cms.djangoapps.contentstore.views.library.LIBRARIES_ENABLED", True) def test_library_creator_status_with_course_staff_role_for_enabled_creator_group_setting(self): _, nostaff_user = self.create_non_staff_authed_user_client() with mock.patch.dict('django.conf.settings.FEATURES', {"ENABLE_CREATOR_GROUP": True}): auth.add_users(self.user, CourseStaffRole(self.course.id), nostaff_user) - self.assertEqual(user_can_create_library(nostaff_user), True) + self.assertEqual(user_can_create_library(nostaff_user, self.course.org), True) + self.assertEqual(user_can_create_library(nostaff_user, 'SomEOtherOrg'), False) # When creator groups are enabled, course instructor members can create libraries + # but only in the org they are course staff for. @mock.patch("cms.djangoapps.contentstore.views.library.LIBRARIES_ENABLED", True) def test_library_creator_status_with_course_instructor_role_for_enabled_creator_group_setting(self): _, nostaff_user = self.create_non_staff_authed_user_client() with mock.patch.dict('django.conf.settings.FEATURES', {"ENABLE_CREATOR_GROUP": True}): auth.add_users(self.user, CourseInstructorRole(self.course.id), nostaff_user) - self.assertEqual(user_can_create_library(nostaff_user), True) + self.assertEqual(user_can_create_library(nostaff_user, self.course.org), True) + self.assertEqual(user_can_create_library(nostaff_user, 'SomEOtherOrg'), False) @ddt.data( (False, False, True), @@ -131,7 +142,7 @@ def test_library_creator_status_settings(self, disable_course, disable_library, "DISABLE_LIBRARY_CREATION": disable_library } ): - self.assertEqual(user_can_create_library(nostaff_user), expected_status) + self.assertEqual(user_can_create_library(nostaff_user, 'SomEOrg'), expected_status) @mock.patch.dict('django.conf.settings.FEATURES', {'DISABLE_COURSE_CREATION': True}) @mock.patch("cms.djangoapps.contentstore.views.library.LIBRARIES_ENABLED", True) @@ -140,7 +151,7 @@ def test_library_creator_status_with_no_course_creator_role_and_disabled_nonstaf Ensure that `DISABLE_COURSE_CREATION` feature works with libraries as well. """ nostaff_client, nostaff_user = self.create_non_staff_authed_user_client() - self.assertFalse(user_can_create_library(nostaff_user)) + self.assertFalse(user_can_create_library(nostaff_user, 'SomEOrg')) # To be explicit, this user can GET, but not POST get_response = nostaff_client.get_json(LIBRARY_REST_URL) @@ -251,7 +262,7 @@ def test_lib_create_permission_course_staff_role(self): auth.add_users(self.user, CourseStaffRole(self.course.id), ns_user) self.assertTrue(auth.user_has_role(ns_user, CourseStaffRole(self.course.id))) response = self.client.ajax_post(LIBRARY_REST_URL, { - 'org': 'org', 'library': 'lib', 'display_name': "New Library", + 'org': self.course.org, 'library': 'lib', 'display_name': "New Library", }) self.assertEqual(response.status_code, 200) From d23b41e2898cda5a19bd2e26ffda854e75bd7829 Mon Sep 17 00:00:00 2001 From: connorhaugh Date: Thu, 15 Feb 2024 21:26:45 +0000 Subject: [PATCH 07/18] docs: imporved comment --- cms/djangoapps/contentstore/views/library.py | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/cms/djangoapps/contentstore/views/library.py b/cms/djangoapps/contentstore/views/library.py index 8ea6bf1e3657..7ba80bcab9f6 100644 --- a/cms/djangoapps/contentstore/views/library.py +++ b/cms/djangoapps/contentstore/views/library.py @@ -98,7 +98,16 @@ def user_can_create_library(user, org): Helper method for returning the library creation status for a particular user, taking into account the value LIBRARIES_ENABLED. - users can only create libraries in orgs they are a part of. + if the ENABLE_CREATOR_GROUP value is False, then any user can create a library (in any org), + if library creation is enabled. + + if the ENABLE_CREATOR_GROUP value is true, then what a user can do varies by thier role. + + Global Staff: can make libraries in any org. + Course Creator Group Members: can make libraries in any org. + Organization Staff: Can make libraries in the organization for which they are staff. + Course Staff: Can make libraries in the organization which has courses of which they are staff. + Course Admin: Can make libraries in the organization which has courses of which they are Admin. """ if org is None: return False From d757cfa2c7564afc479b2dfdab25b57adc068109 Mon Sep 17 00:00:00 2001 From: connorhaugh Date: Wed, 4 Oct 2023 12:46:36 -0400 Subject: [PATCH 08/18] fix: cohorts data can be private --- common/djangoapps/util/file.py | 11 ++++++++++- lms/djangoapps/instructor/views/api.py | 3 ++- 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/common/djangoapps/util/file.py b/common/djangoapps/util/file.py index 66fc2a6f5f35..b884ca46a703 100644 --- a/common/djangoapps/util/file.py +++ b/common/djangoapps/util/file.py @@ -13,6 +13,7 @@ from django.utils.translation import gettext as _ from django.utils.translation import ngettext from pytz import UTC +from storages.backends.s3boto3 import S3Boto3Storage class FileValidationException(Exception): @@ -23,7 +24,7 @@ class FileValidationException(Exception): def store_uploaded_file( - request, file_key, allowed_file_types, base_storage_filename, max_file_size, validator=None, + request, file_key, allowed_file_types, base_storage_filename, max_file_size, validator=None, is_private=False, ): """ Stores an uploaded file to django file storage. @@ -45,6 +46,8 @@ def store_uploaded_file( a `FileValidationException` if the file is not properly formatted. If any exception is thrown, the stored file will be deleted before the exception is re-raised. Note that the implementor of the validator function should take care to close the stored file if they open it for reading. + is_private (Boolean): an optional boolean which if True and the storage backend is S3, + sets the ACL for the file object to be private. Returns: Storage: the file storage object where the file can be retrieved from @@ -75,6 +78,12 @@ def store_uploaded_file( file_storage = DefaultStorage() # If a file already exists with the supplied name, file_storage will make the filename unique. stored_file_name = file_storage.save(stored_file_name, uploaded_file) + if is_private and settings.DEFAULT_FILE_STORAGE == 'storages.backends.s3boto3.S3Boto3Storage': + S3Boto3Storage().connection.meta.client.put_object_acl( ++ ACL='private', ++ Bucket=settings.AWS_STORAGE_BUCKET_NAME, ++ Key=stored_file_name, ++ ) if validator: try: diff --git a/lms/djangoapps/instructor/views/api.py b/lms/djangoapps/instructor/views/api.py index 6d7dfe17a9d7..02a91e7d84de 100644 --- a/lms/djangoapps/instructor/views/api.py +++ b/lms/djangoapps/instructor/views/api.py @@ -1603,7 +1603,8 @@ def post(self, request, course_key_string): request, 'uploaded-file', ['.csv'], course_and_time_based_filename_generator(course_key, 'cohorts'), max_file_size=2000000, # limit to 2 MB - validator=_cohorts_csv_validator + validator=_cohorts_csv_validator, + is_private=True ) task_api.submit_cohort_students(request, course_key, file_name) except (FileValidationException, ValueError) as e: From 62269f8c956e25f36d83e06299a90018a535b79f Mon Sep 17 00:00:00 2001 From: Feanil Patel Date: Thu, 25 Jul 2024 10:24:36 -0400 Subject: [PATCH 09/18] fix: Remove errant pluses from a bad merge. --- common/djangoapps/util/file.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/common/djangoapps/util/file.py b/common/djangoapps/util/file.py index b884ca46a703..b2892e6f42c9 100644 --- a/common/djangoapps/util/file.py +++ b/common/djangoapps/util/file.py @@ -80,10 +80,10 @@ def store_uploaded_file( stored_file_name = file_storage.save(stored_file_name, uploaded_file) if is_private and settings.DEFAULT_FILE_STORAGE == 'storages.backends.s3boto3.S3Boto3Storage': S3Boto3Storage().connection.meta.client.put_object_acl( -+ ACL='private', -+ Bucket=settings.AWS_STORAGE_BUCKET_NAME, -+ Key=stored_file_name, -+ ) + ACL='private', + Bucket=settings.AWS_STORAGE_BUCKET_NAME, + Key=stored_file_name, + ) if validator: try: From 8813e8b02cf30f2e5d619db72c4a2cdd80834eb8 Mon Sep 17 00:00:00 2001 From: Feanil Patel Date: Thu, 25 Jul 2024 10:49:14 -0400 Subject: [PATCH 10/18] style: Fix a pylint and style violations. --- cms/djangoapps/contentstore/utils.py | 3 ++- cms/djangoapps/contentstore/views/library.py | 2 +- cms/djangoapps/contentstore/views/tests/test_library.py | 2 +- 3 files changed, 4 insertions(+), 3 deletions(-) diff --git a/cms/djangoapps/contentstore/utils.py b/cms/djangoapps/contentstore/utils.py index 1b5af4f769bd..21f605a9e440 100644 --- a/cms/djangoapps/contentstore/utils.py +++ b/cms/djangoapps/contentstore/utils.py @@ -1699,7 +1699,8 @@ def get_home_context(request, no_course=False): 'library_authoring_mfe_url': LIBRARY_AUTHORING_MICROFRONTEND_URL, 'taxonomy_list_mfe_url': get_taxonomy_list_url(), 'libraries': libraries, - 'show_new_library_button': user_can_view_create_library_button(user) and not should_redirect_to_library_authoring_mfe(), + 'show_new_library_button': user_can_view_create_library_button(user) + and not should_redirect_to_library_authoring_mfe(), 'user': user, 'request_course_creator_url': reverse('request_course_creator'), 'course_creator_status': _get_course_creator_status(user), diff --git a/cms/djangoapps/contentstore/views/library.py b/cms/djangoapps/contentstore/views/library.py index 7ba80bcab9f6..870c192653d2 100644 --- a/cms/djangoapps/contentstore/views/library.py +++ b/cms/djangoapps/contentstore/views/library.py @@ -68,6 +68,7 @@ def should_redirect_to_library_authoring_mfe(): REDIRECT_TO_LIBRARY_AUTHORING_MICROFRONTEND.is_enabled() ) + def user_can_view_create_library_button(user): """ Helper method for displaying the visibilty of the create_library_button. @@ -92,7 +93,6 @@ def user_can_view_create_library_button(user): return not disable_course_creation - def user_can_create_library(user, org): """ Helper method for returning the library creation status for a particular user, diff --git a/cms/djangoapps/contentstore/views/tests/test_library.py b/cms/djangoapps/contentstore/views/tests/test_library.py index fdbb9d905c62..fa6505419725 100644 --- a/cms/djangoapps/contentstore/views/tests/test_library.py +++ b/cms/djangoapps/contentstore/views/tests/test_library.py @@ -111,7 +111,7 @@ def test_library_creator_status_with_course_staff_role_for_enabled_creator_group self.assertEqual(user_can_create_library(nostaff_user, 'SomEOtherOrg'), False) # When creator groups are enabled, course instructor members can create libraries - # but only in the org they are course staff for. + # but only in the org they are course staff for. @mock.patch("cms.djangoapps.contentstore.views.library.LIBRARIES_ENABLED", True) def test_library_creator_status_with_course_instructor_role_for_enabled_creator_group_setting(self): _, nostaff_user = self.create_non_staff_authed_user_client() From 1e0d575f5ff686ccc600e1efc1aab9e812a059ee Mon Sep 17 00:00:00 2001 From: Fateme Khodayari <55655542+FatemeKhodayari@users.noreply.github.com> Date: Tue, 30 Jul 2024 18:16:09 +0330 Subject: [PATCH 11/18] fix: course progress url returned based on course_home_mfe_progress_tab_is_active (#35149) --- lms/djangoapps/learner_home/serializers.py | 3 ++- .../learner_home/test_serializers.py | 26 ++++++++++++++++++- lms/djangoapps/learner_home/utils.py | 16 ++++++++++++ 3 files changed, 43 insertions(+), 2 deletions(-) diff --git a/lms/djangoapps/learner_home/serializers.py b/lms/djangoapps/learner_home/serializers.py index 0ce7bf9c6977..b3471715b9dc 100644 --- a/lms/djangoapps/learner_home/serializers.py +++ b/lms/djangoapps/learner_home/serializers.py @@ -15,6 +15,7 @@ from common.djangoapps.course_modes.models import CourseMode from openedx.features.course_experience import course_home_url from xmodule.data import CertificatesDisplayBehaviors +from lms.djangoapps.learner_home.utils import course_progress_url class LiteralField(serializers.Field): @@ -116,7 +117,7 @@ def get_homeUrl(self, instance): return course_home_url(instance.course_id) def get_progressUrl(self, instance): - return reverse("progress", kwargs={"course_id": instance.course_id}) + return course_progress_url(instance.course_id) def get_unenrollUrl(self, instance): return reverse("course_run_refund_status", args=[instance.course_id]) diff --git a/lms/djangoapps/learner_home/test_serializers.py b/lms/djangoapps/learner_home/test_serializers.py index f588af58aee4..ac11a8b2990d 100644 --- a/lms/djangoapps/learner_home/test_serializers.py +++ b/lms/djangoapps/learner_home/test_serializers.py @@ -51,7 +51,7 @@ SuggestedCourseSerializer, UnfulfilledEntitlementSerializer, ) - +from lms.djangoapps.learner_home.utils import course_progress_url from lms.djangoapps.learner_home.test_utils import ( datetime_to_django_format, random_bool, @@ -224,6 +224,30 @@ def test_missing_resume_url(self): # Then the resumeUrl is None, which is allowed self.assertIsNone(output_data["resumeUrl"]) + def is_progress_url_matching_course_home_mfe_progress_tab_is_active(self): + """ + Compares the progress URL generated by CourseRunSerializer to the expected progress URL. + + :return: True if the generated progress URL matches the expected, False otherwise. + """ + input_data = self.create_test_enrollment() + input_context = self.create_test_context(input_data.course.id) + output_data = CourseRunSerializer(input_data, context=input_context).data + return output_data['progressUrl'] == course_progress_url(input_data.course.id) + + @mock.patch('lms.djangoapps.learner_home.utils.course_home_mfe_progress_tab_is_active') + def test_progress_url(self, mock_course_home_mfe_progress_tab_is_active): + """ + Tests the progress URL generated by the CourseRunSerializer. When course_home_mfe_progress_tab_is_active + is true, the generated progress URL must point to the progress page of the course home (learning) MFE. + Otherwise, it must point to the legacy progress page. + """ + mock_course_home_mfe_progress_tab_is_active.return_value = True + self.assertTrue(self.is_progress_url_matching_course_home_mfe_progress_tab_is_active()) + + mock_course_home_mfe_progress_tab_is_active.return_value = False + self.assertTrue(self.is_progress_url_matching_course_home_mfe_progress_tab_is_active()) + @ddt.ddt class TestCoursewareAccessSerializer(LearnerDashboardBaseTest): diff --git a/lms/djangoapps/learner_home/utils.py b/lms/djangoapps/learner_home/utils.py index 28e4479f9439..96af6a64452b 100644 --- a/lms/djangoapps/learner_home/utils.py +++ b/lms/djangoapps/learner_home/utils.py @@ -4,6 +4,7 @@ import logging +from django.urls import reverse from django.contrib.auth import get_user_model from django.core.exceptions import MultipleObjectsReturned from rest_framework.exceptions import PermissionDenied, NotFound @@ -11,6 +12,8 @@ from common.djangoapps.student.models import ( get_user_by_username_or_email, ) +from lms.djangoapps.course_home_api.toggles import course_home_mfe_progress_tab_is_active +from openedx.features.course_experience.url_helpers import get_learning_mfe_home_url log = logging.getLogger(__name__) User = get_user_model() @@ -54,3 +57,16 @@ def get_masquerade_user(request): ) log.info(success_msg) return masquerade_user + + +def course_progress_url(course_key) -> str: + """ + Returns the course progress page's URL for the current user. + + :param course_key: The course key for which the home url is being requested. + + :return: The course progress page URL. + """ + if course_home_mfe_progress_tab_is_active(course_key): + return get_learning_mfe_home_url(course_key, url_fragment='progress') + return reverse('progress', kwargs={'course_id': course_key}) From c5d750713e006cf9577caffb81ae89167fdbd0f5 Mon Sep 17 00:00:00 2001 From: Mariagabriela Jaimes Date: Tue, 6 Aug 2024 18:11:47 -0400 Subject: [PATCH 12/18] chore: upgrade Django to 4.2.15 (#35240) --- requirements/edx/base.txt | 2 +- requirements/edx/development.txt | 2 +- requirements/edx/doc.txt | 2 +- requirements/edx/testing.txt | 2 +- scripts/user_retirement/requirements/base.txt | 2 +- scripts/user_retirement/requirements/testing.txt | 2 +- 6 files changed, 6 insertions(+), 6 deletions(-) diff --git a/requirements/edx/base.txt b/requirements/edx/base.txt index d68c66cc6ef4..1ef346dd0380 100644 --- a/requirements/edx/base.txt +++ b/requirements/edx/base.txt @@ -177,7 +177,7 @@ defusedxml==0.7.1 # ora2 # python3-openid # social-auth-core -django==4.2.14 +django==4.2.15 # via # -c requirements/edx/../common_constraints.txt # -c requirements/edx/../constraints.txt diff --git a/requirements/edx/development.txt b/requirements/edx/development.txt index f78a27d03813..6bd8eaaa7671 100644 --- a/requirements/edx/development.txt +++ b/requirements/edx/development.txt @@ -347,7 +347,7 @@ distlib==0.3.8 # via # -r requirements/edx/testing.txt # virtualenv -django==4.2.14 +django==4.2.15 # via # -c requirements/edx/../common_constraints.txt # -c requirements/edx/../constraints.txt diff --git a/requirements/edx/doc.txt b/requirements/edx/doc.txt index de65c64de287..eda0c2ef66d0 100644 --- a/requirements/edx/doc.txt +++ b/requirements/edx/doc.txt @@ -227,7 +227,7 @@ defusedxml==0.7.1 # ora2 # python3-openid # social-auth-core -django==4.2.14 +django==4.2.15 # via # -c requirements/edx/../common_constraints.txt # -c requirements/edx/../constraints.txt diff --git a/requirements/edx/testing.txt b/requirements/edx/testing.txt index 0baf5e170300..93ea32b23099 100644 --- a/requirements/edx/testing.txt +++ b/requirements/edx/testing.txt @@ -263,7 +263,7 @@ dill==0.3.8 # via pylint distlib==0.3.8 # via virtualenv -django==4.2.14 +django==4.2.15 # via # -c requirements/edx/../common_constraints.txt # -c requirements/edx/../constraints.txt diff --git a/scripts/user_retirement/requirements/base.txt b/scripts/user_retirement/requirements/base.txt index 1e510e645926..eff125066bc4 100644 --- a/scripts/user_retirement/requirements/base.txt +++ b/scripts/user_retirement/requirements/base.txt @@ -39,7 +39,7 @@ click==8.1.6 # edx-django-utils cryptography==42.0.7 # via pyjwt -django==4.2.14 +django==4.2.15 # via # -c scripts/user_retirement/requirements/../../../requirements/common_constraints.txt # -c scripts/user_retirement/requirements/../../../requirements/constraints.txt diff --git a/scripts/user_retirement/requirements/testing.txt b/scripts/user_retirement/requirements/testing.txt index 15cc2d9849bf..712e1ba73463 100644 --- a/scripts/user_retirement/requirements/testing.txt +++ b/scripts/user_retirement/requirements/testing.txt @@ -56,7 +56,7 @@ cryptography==42.0.7 # pyjwt ddt==1.7.2 # via -r scripts/user_retirement/requirements/testing.in -django==4.2.14 +django==4.2.15 # via # -r scripts/user_retirement/requirements/base.txt # django-crum From d5c84e9d21b407717e6b6b47551e36f4b0605cc8 Mon Sep 17 00:00:00 2001 From: Muhammad Anas <88967643+Anas12091101@users.noreply.github.com> Date: Thu, 8 Aug 2024 11:12:19 +0500 Subject: [PATCH 13/18] backport fix: disable submit button for archived courses (#34920) to redwood (#35248) * fix: disable submit button for archived courses (#34920) --- xmodule/capa_block.py | 15 ++++++++++++++- xmodule/tests/test_capa_block.py | 33 +++++++++++++++++++++++++++++++- 2 files changed, 46 insertions(+), 2 deletions(-) diff --git a/xmodule/capa_block.py b/xmodule/capa_block.py index e7d917fee601..5779ed367fe8 100644 --- a/xmodule/capa_block.py +++ b/xmodule/capa_block.py @@ -795,12 +795,25 @@ def generate_report_data(self, user_state_iterator, limit_responses=None): } yield (user_state.username, report) + @property + def course_end_date(self): + """ + Return the end date of the problem's course + """ + + try: + course_block_key = self.runtime.course_entry.structure['root'] + return self.runtime.course_entry.structure['blocks'][course_block_key].fields['end'] + except (AttributeError, KeyError): + return None + @property def close_date(self): """ Return the date submissions should be closed from. """ - due_date = self.due + + due_date = self.due or self.course_end_date if self.graceperiod is not None and due_date: return due_date + self.graceperiod diff --git a/xmodule/tests/test_capa_block.py b/xmodule/tests/test_capa_block.py index 42494547822e..5da29ac3d20e 100644 --- a/xmodule/tests/test_capa_block.py +++ b/xmodule/tests/test_capa_block.py @@ -11,7 +11,7 @@ import random import textwrap import unittest -from unittest.mock import DEFAULT, Mock, patch +from unittest.mock import DEFAULT, Mock, patch, PropertyMock import pytest import ddt @@ -656,6 +656,37 @@ def test_closed(self): due=self.yesterday_str) assert block.closed() + @patch.object(ProblemBlock, 'course_end_date', new_callable=PropertyMock) + def test_closed_for_archive(self, mock_course_end_date): + + # Utility to create a datetime object in the past + def past_datetime(days): + return (datetime.datetime.now(UTC) - datetime.timedelta(days=days)) + + # Utility to create a datetime object in the future + def future_datetime(days): + return (datetime.datetime.now(UTC) + datetime.timedelta(days=days)) + + block = CapaFactory.create(max_attempts="1", attempts="0") + + # For active courses without graceperiod + mock_course_end_date.return_value = future_datetime(10) + assert not block.closed() + + # For archive courses without graceperiod + mock_course_end_date.return_value = past_datetime(10) + assert block.closed() + + # For active courses with graceperiod + mock_course_end_date.return_value = future_datetime(10) + block.graceperiod = datetime.timedelta(days=2) + assert not block.closed() + + # For archive courses with graceperiod + mock_course_end_date.return_value = past_datetime(2) + block.graceperiod = datetime.timedelta(days=3) + assert not block.closed() + def test_parse_get_params(self): # Valid GET param dict From fa97f13196bf920286c1122f71c726c31427099d Mon Sep 17 00:00:00 2001 From: Tim McCormack Date: Wed, 31 Jul 2024 17:36:08 +0000 Subject: [PATCH 14/18] fix: Prevent error page recursion (#35209) We sometimes see rendering errors in the error page itself, which then cause another attempt at rendering the error page. I'm not sure _exactly_ how the loop is occurring, but it looks something like this: 1. An error is raised in a view or middleware and is not caught by application code 2. Django catches the error and calls the registered uncaught error handler 3. Our handler tries to render an error page 4. The rendering code raises an error 5. GOTO 2 (until some sort of server limit is reached) By catching all errors raised during error-page render and substituting in a hardcoded string, we can reduce server resources, avoid logging massive sequences of recursive stack traces, and still give the user *some* indication that yes, there was a problem. This should help address https://github.com/openedx/edx-platform/issues/35151 At least one of these rendering errors is known to be due to a translation error. There's a separate issue for restoring translation quality so that we avoid those issues in the future (https://github.com/openedx/openedx-translations/issues/549) but in general we should catch all rendering errors, including unknown ones. Testing: - In `lms/envs/devstack.py` change `DEBUG` to `False` to ensure that the usual error page is displayed (rather than the debug error page). - Add line `1/0` to the top of the `student_dashboard` function in `common/djangoapps/student/views/dashboard.py` to make that view error. - In `lms/templates/static_templates/server-error.html` replace `static.get_platform_name()` with `None * 7` to make the error template itself produce an error. - Visit . Without the fix, the response takes 10 seconds and produces a 6 MB, 85k line set of stack traces and the page displays "A server error occurred. Please contact the administrator." With the fix, the response takes less than a second and produces three stack traces (one of which contains the error page's rendering error). --- lms/djangoapps/static_template_view/views.py | 23 ++++++++++++++++++-- 1 file changed, 21 insertions(+), 2 deletions(-) diff --git a/lms/djangoapps/static_template_view/views.py b/lms/djangoapps/static_template_view/views.py index 01b99d51c861..a788f77a95fd 100644 --- a/lms/djangoapps/static_template_view/views.py +++ b/lms/djangoapps/static_template_view/views.py @@ -5,7 +5,7 @@ # List of valid templates is explicitly managed for (short-term) # security reasons. - +import logging import mimetypes from django.conf import settings @@ -23,6 +23,8 @@ from common.djangoapps.util.views import fix_crum_request from openedx.core.djangoapps.site_configuration import helpers as configuration_helpers +log = logging.getLogger(__name__) + valid_templates = [] if settings.STATIC_GRAB: @@ -122,4 +124,21 @@ def render_429(request, exception=None): # lint-amnesty, pylint: disable=unused @fix_crum_request def render_500(request): - return HttpResponseServerError(render_to_string('static_templates/server-error.html', {}, request=request)) + """ + Render the generic error page when we have an uncaught error. + """ + try: + return HttpResponseServerError(render_to_string('static_templates/server-error.html', {}, request=request)) + except BaseException as e: + # If we can't render the error page, ensure we don't raise another + # exception -- because if we do, we'll probably just end up back + # at the same rendering error. + # + # This is an attempt at working around the recursive error handling issues + # observed in , which + # were triggered by Mako and translation errors. + + log.error("Encountered error while rendering error page.", exc_info=True) + # This message is intentionally hardcoded and does not involve + # any translation, templating, etc. Do not translate. + return HttpResponseServerError("Encountered error while rendering error page.") From 5ea3b9851238ef886e215e00eb0fd6394fbcd6ed Mon Sep 17 00:00:00 2001 From: Ihor Romaniuk Date: Mon, 9 Sep 2024 21:30:58 +0200 Subject: [PATCH 15/18] feat: course about page markup and styles improvements (#34892) * feat: course about page markup and styles improvements * test: update tests according to changes * fix: relocate course organization and return removed prerequisites info * fix: display org info above the course title --------- Co-authored-by: oleksandr.buhaienko Co-authored-by: Eugene Dyudyunov --- lms/djangoapps/courseware/tests/test_about.py | 10 ++++++-- .../sass/multicourse/_course_about.scss | 24 +++++++++++++++++++ lms/templates/courseware/course_about.html | 19 ++++++++++----- 3 files changed, 45 insertions(+), 8 deletions(-) diff --git a/lms/djangoapps/courseware/tests/test_about.py b/lms/djangoapps/courseware/tests/test_about.py index d53d620d3e34..bd0c1854ab76 100644 --- a/lms/djangoapps/courseware/tests/test_about.py +++ b/lms/djangoapps/courseware/tests/test_about.py @@ -156,7 +156,10 @@ def test_pre_requisite_course(self): assert resp.status_code == 200 pre_requisite_courses = get_prerequisite_courses_display(course) pre_requisite_course_about_url = reverse('about_course', args=[str(pre_requisite_courses[0]['key'])]) - assert '{}'.format(pre_requisite_course_about_url, pre_requisite_courses[0]['display']) in resp.content.decode(resp.charset).strip('\n') # pylint: disable=line-too-long + assert ( + f'You must successfully complete ' + f'{pre_requisite_courses[0]["display"]} before you begin this course.' + ) in resp.content.decode(resp.charset).strip('\n') @patch.dict(settings.FEATURES, {'ENABLE_PREREQUISITE_COURSES': True}) def test_about_page_unfulfilled_prereqs(self): @@ -190,7 +193,10 @@ def test_about_page_unfulfilled_prereqs(self): assert resp.status_code == 200 pre_requisite_courses = get_prerequisite_courses_display(course) pre_requisite_course_about_url = reverse('about_course', args=[str(pre_requisite_courses[0]['key'])]) - assert '{}'.format(pre_requisite_course_about_url, pre_requisite_courses[0]['display']) in resp.content.decode(resp.charset).strip('\n') # pylint: disable=line-too-long + assert ( + f'You must successfully complete ' + f'{pre_requisite_courses[0]["display"]} before you begin this course.' + ) in resp.content.decode(resp.charset).strip('\n') url = reverse('about_course', args=[str(pre_requisite_course.id)]) resp = self.client.get(url) diff --git a/lms/static/sass/multicourse/_course_about.scss b/lms/static/sass/multicourse/_course_about.scss index 629b3065778e..a8a34ccec589 100644 --- a/lms/static/sass/multicourse/_course_about.scss +++ b/lms/static/sass/multicourse/_course_about.scss @@ -44,6 +44,11 @@ > div.table { display: table; width: 100%; + + @include media-breakpoint-down(sm) { + display: flex; + flex-direction: column; + } } .intro { @@ -51,6 +56,11 @@ @include clearfix(); + @include media-breakpoint-down(sm) { + width: auto; + order: 2; + } + display: table-cell; vertical-align: middle; padding: $baseline; @@ -127,6 +137,10 @@ a.add-to-cart { @include button(shiny, $button-color); + @include media-breakpoint-down(md) { + width: 100%; + } + box-sizing: border-box; border-radius: 3px; display: block; @@ -189,6 +203,11 @@ @include float(left); @include margin(1px, flex-gutter(8), 0, 0); @include transition(none); + @include media-breakpoint-down(md) { + width: 100%; + margin-right: 0; + margin-bottom: 10px; + } width: flex-grid(5, 8); } @@ -213,6 +232,11 @@ width: flex-grid(4); z-index: 2; + @include media-breakpoint-down(sm) { + width: auto; + order: 1; + } + .hero { border: 1px solid $border-color-3; height: 100%; diff --git a/lms/templates/courseware/course_about.html b/lms/templates/courseware/course_about.html index 91d7a2a28e57..eec9caeadbec 100644 --- a/lms/templates/courseware/course_about.html +++ b/lms/templates/courseware/course_about.html @@ -62,11 +62,10 @@
-

- ${course.display_name_with_default} -

+

${course.display_org_with_default}

+

${course.display_name_with_default}


- ${course.display_org_with_default} +

${get_course_about_section(request, course, 'short_description')}

@@ -160,7 +159,11 @@

<%block name="course_about_important_dates">
    -

  1. ${_("Course Number")}

    ${course.display_number_with_default}
    2. +
  2. + +

    ${_("Course Number")}

    + ${course.display_number_with_default} +
    3. % if not course.start_date_is_still_default: <% course_start_date = course.advertised_start or course.start @@ -231,7 +234,11 @@

    % endif % if get_course_about_section(request, course, "prerequisites"): -

  3. ${_("Requirements")}

    ${get_course_about_section(request, course, "prerequisites")}
    4. +
  4. + +

    ${_("Requirements")}

    + ${get_course_about_section(request, course, "prerequisites")} +
    5. % endif

From 9c8059b93386e9f1f4a4d0ed51038cf88d8036c4 Mon Sep 17 00:00:00 2001 From: Kaustav Banerjee Date: Thu, 19 Sep 2024 21:52:58 +0530 Subject: [PATCH 16/18] fix: backport: hide new library button for ineligible users in split studio view (#35488) --- cms/djangoapps/contentstore/utils.py | 3 +- cms/djangoapps/contentstore/views/library.py | 56 +++++++++----------- 2 files changed, 26 insertions(+), 33 deletions(-) diff --git a/cms/djangoapps/contentstore/utils.py b/cms/djangoapps/contentstore/utils.py index 21f605a9e440..ad68ea0c4318 100644 --- a/cms/djangoapps/contentstore/utils.py +++ b/cms/djangoapps/contentstore/utils.py @@ -1534,6 +1534,7 @@ def get_library_context(request, request_is_json=False): ) from cms.djangoapps.contentstore.views.library import ( LIBRARIES_ENABLED, + user_can_view_create_library_button, ) libraries = _accessible_libraries_iter(request.user) if LIBRARIES_ENABLED else [] @@ -1547,7 +1548,7 @@ def get_library_context(request, request_is_json=False): 'in_process_course_actions': [], 'courses': [], 'libraries_enabled': LIBRARIES_ENABLED, - 'show_new_library_button': LIBRARIES_ENABLED and request.user.is_active, + 'show_new_library_button': user_can_view_create_library_button(request.user) and request.user.is_active, 'user': request.user, 'request_course_creator_url': reverse('request_course_creator'), 'course_creator_status': _get_course_creator_status(request.user), diff --git a/cms/djangoapps/contentstore/views/library.py b/cms/djangoapps/contentstore/views/library.py index 870c192653d2..8c314caa6697 100644 --- a/cms/djangoapps/contentstore/views/library.py +++ b/cms/djangoapps/contentstore/views/library.py @@ -69,31 +69,7 @@ def should_redirect_to_library_authoring_mfe(): ) -def user_can_view_create_library_button(user): - """ - Helper method for displaying the visibilty of the create_library_button. - """ - if not LIBRARIES_ENABLED: - return False - elif user.is_staff: - return True - elif settings.FEATURES.get('ENABLE_CREATOR_GROUP', False): - is_course_creator = get_course_creator_status(user) == 'granted' - has_org_staff_role = OrgStaffRole().get_orgs_for_user(user).exists() - has_course_staff_role = UserBasedRole(user=user, role=CourseStaffRole.ROLE).courses_with_role().exists() - has_course_admin_role = UserBasedRole(user=user, role=CourseInstructorRole.ROLE).courses_with_role().exists() - return is_course_creator or has_org_staff_role or has_course_staff_role or has_course_admin_role - else: - # EDUCATOR-1924: DISABLE_LIBRARY_CREATION overrides DISABLE_COURSE_CREATION, if present. - disable_library_creation = settings.FEATURES.get('DISABLE_LIBRARY_CREATION', None) - disable_course_creation = settings.FEATURES.get('DISABLE_COURSE_CREATION', False) - if disable_library_creation is not None: - return not disable_library_creation - else: - return not disable_course_creation - - -def user_can_create_library(user, org): +def _user_can_create_library_for_org(user, org=None): """ Helper method for returning the library creation status for a particular user, taking into account the value LIBRARIES_ENABLED. @@ -109,29 +85,29 @@ def user_can_create_library(user, org): Course Staff: Can make libraries in the organization which has courses of which they are staff. Course Admin: Can make libraries in the organization which has courses of which they are Admin. """ - if org is None: - return False if not LIBRARIES_ENABLED: return False elif user.is_staff: return True - if settings.FEATURES.get('ENABLE_CREATOR_GROUP', False): + elif settings.FEATURES.get('ENABLE_CREATOR_GROUP', False): + org_filter_params = {} + if org: + org_filter_params['org'] = org is_course_creator = get_course_creator_status(user) == 'granted' - has_org_staff_role = org in OrgStaffRole().get_orgs_for_user(user) + has_org_staff_role = OrgStaffRole().get_orgs_for_user(user).filter(**org_filter_params).exists() has_course_staff_role = ( UserBasedRole(user=user, role=CourseStaffRole.ROLE) .courses_with_role() - .filter(org=org) + .filter(**org_filter_params) .exists() ) has_course_admin_role = ( UserBasedRole(user=user, role=CourseInstructorRole.ROLE) .courses_with_role() - .filter(org=org) + .filter(**org_filter_params) .exists() ) return is_course_creator or has_org_staff_role or has_course_staff_role or has_course_admin_role - else: # EDUCATOR-1924: DISABLE_LIBRARY_CREATION overrides DISABLE_COURSE_CREATION, if present. disable_library_creation = settings.FEATURES.get('DISABLE_LIBRARY_CREATION', None) @@ -142,6 +118,22 @@ def user_can_create_library(user, org): return not disable_course_creation +def user_can_view_create_library_button(user): + """ + Helper method for displaying the visibilty of the create_library_button. + """ + return _user_can_create_library_for_org(user) + + +def user_can_create_library(user, org): + """ + Helper method for to check if user can create library for given org. + """ + if org is None: + return False + return _user_can_create_library_for_org(user, org) + + @login_required @ensure_csrf_cookie @require_http_methods(('GET', 'POST')) From 9ef785a3d9b33f41305c18434dc1e3315df69d20 Mon Sep 17 00:00:00 2001 From: magajh Date: Tue, 10 Sep 2024 05:21:07 -0400 Subject: [PATCH 17/18] chore: upgrade Django to 4.2.16 --- requirements/edx/base.txt | 2 +- requirements/edx/development.txt | 2 +- requirements/edx/doc.txt | 2 +- requirements/edx/testing.txt | 2 +- scripts/user_retirement/requirements/base.txt | 2 +- scripts/user_retirement/requirements/testing.txt | 2 +- 6 files changed, 6 insertions(+), 6 deletions(-) diff --git a/requirements/edx/base.txt b/requirements/edx/base.txt index 1ef346dd0380..49cd864938a3 100644 --- a/requirements/edx/base.txt +++ b/requirements/edx/base.txt @@ -177,7 +177,7 @@ defusedxml==0.7.1 # ora2 # python3-openid # social-auth-core -django==4.2.15 +django==4.2.16 # via # -c requirements/edx/../common_constraints.txt # -c requirements/edx/../constraints.txt diff --git a/requirements/edx/development.txt b/requirements/edx/development.txt index 6bd8eaaa7671..6970e72d8351 100644 --- a/requirements/edx/development.txt +++ b/requirements/edx/development.txt @@ -347,7 +347,7 @@ distlib==0.3.8 # via # -r requirements/edx/testing.txt # virtualenv -django==4.2.15 +django==4.2.16 # via # -c requirements/edx/../common_constraints.txt # -c requirements/edx/../constraints.txt diff --git a/requirements/edx/doc.txt b/requirements/edx/doc.txt index eda0c2ef66d0..c942cda4741a 100644 --- a/requirements/edx/doc.txt +++ b/requirements/edx/doc.txt @@ -227,7 +227,7 @@ defusedxml==0.7.1 # ora2 # python3-openid # social-auth-core -django==4.2.15 +django==4.2.16 # via # -c requirements/edx/../common_constraints.txt # -c requirements/edx/../constraints.txt diff --git a/requirements/edx/testing.txt b/requirements/edx/testing.txt index 93ea32b23099..40109175905f 100644 --- a/requirements/edx/testing.txt +++ b/requirements/edx/testing.txt @@ -263,7 +263,7 @@ dill==0.3.8 # via pylint distlib==0.3.8 # via virtualenv -django==4.2.15 +django==4.2.16 # via # -c requirements/edx/../common_constraints.txt # -c requirements/edx/../constraints.txt diff --git a/scripts/user_retirement/requirements/base.txt b/scripts/user_retirement/requirements/base.txt index eff125066bc4..1966c7bb8b05 100644 --- a/scripts/user_retirement/requirements/base.txt +++ b/scripts/user_retirement/requirements/base.txt @@ -39,7 +39,7 @@ click==8.1.6 # edx-django-utils cryptography==42.0.7 # via pyjwt -django==4.2.15 +django==4.2.16 # via # -c scripts/user_retirement/requirements/../../../requirements/common_constraints.txt # -c scripts/user_retirement/requirements/../../../requirements/constraints.txt diff --git a/scripts/user_retirement/requirements/testing.txt b/scripts/user_retirement/requirements/testing.txt index 712e1ba73463..823b1d66887c 100644 --- a/scripts/user_retirement/requirements/testing.txt +++ b/scripts/user_retirement/requirements/testing.txt @@ -56,7 +56,7 @@ cryptography==42.0.7 # pyjwt ddt==1.7.2 # via -r scripts/user_retirement/requirements/testing.in -django==4.2.15 +django==4.2.16 # via # -r scripts/user_retirement/requirements/base.txt # django-crum From daf696a9bf8ab9eeec8170a2905db2cb3569d826 Mon Sep 17 00:00:00 2001 From: magajh Date: Tue, 8 Oct 2024 22:55:06 -0400 Subject: [PATCH 18/18] chore: compile requirements --- requirements/common_constraints.txt | 9 --------- requirements/edx/base.txt | 4 +--- requirements/edx/development.txt | 1 - requirements/edx/doc.txt | 1 - requirements/edx/testing.txt | 1 - requirements/pip-tools.txt | 4 +--- 6 files changed, 2 insertions(+), 18 deletions(-) diff --git a/requirements/common_constraints.txt b/requirements/common_constraints.txt index ef8bc86061b7..605871970bd8 100644 --- a/requirements/common_constraints.txt +++ b/requirements/common_constraints.txt @@ -26,15 +26,6 @@ elasticsearch<7.14.0 # django-simple-history>3.0.0 adds indexing and causes a lot of migrations to be affected -# opentelemetry requires version 6.x at the moment: -# https://github.com/open-telemetry/opentelemetry-python/issues/3570 -# Normally this could be added as a constraint in edx-django-utils, where we're -# adding the opentelemetry dependency. However, when we compile pip-tools.txt, -# that uses version 7.x, and then there's no undoing that when compiling base.txt. -# So we need to pin it globally, for now. -# Ticket for unpinning: https://github.com/openedx/edx-lint/issues/407 -importlib-metadata<7 - # Cause: https://github.com/openedx/event-tracking/pull/290 # event-tracking 2.4.1 upgrades to pymongo 4.4.0 which is not supported on edx-platform. # We will pin event-tracking to do not break existing installations diff --git a/requirements/edx/base.txt b/requirements/edx/base.txt index 49cd864938a3..4ca96ccfe951 100644 --- a/requirements/edx/base.txt +++ b/requirements/edx/base.txt @@ -614,9 +614,7 @@ idna==3.7 # snowflake-connector-python # yarl importlib-metadata==6.11.0 - # via - # -c requirements/edx/../common_constraints.txt - # markdown + # via markdown importlib-resources==5.13.0 # via # jsonschema diff --git a/requirements/edx/development.txt b/requirements/edx/development.txt index 6970e72d8351..5d5bf7c27147 100644 --- a/requirements/edx/development.txt +++ b/requirements/edx/development.txt @@ -1035,7 +1035,6 @@ import-linter==2.0 # via -r requirements/edx/testing.txt importlib-metadata==6.11.0 # via - # -c requirements/edx/../common_constraints.txt # -r requirements/edx/../pip-tools.txt # -r requirements/edx/doc.txt # -r requirements/edx/testing.txt diff --git a/requirements/edx/doc.txt b/requirements/edx/doc.txt index c942cda4741a..efe2fdc4dfc4 100644 --- a/requirements/edx/doc.txt +++ b/requirements/edx/doc.txt @@ -712,7 +712,6 @@ imagesize==1.4.1 # via sphinx importlib-metadata==6.11.0 # via - # -c requirements/edx/../common_constraints.txt # -r requirements/edx/base.txt # markdown # sphinx diff --git a/requirements/edx/testing.txt b/requirements/edx/testing.txt index 40109175905f..522f119908c3 100644 --- a/requirements/edx/testing.txt +++ b/requirements/edx/testing.txt @@ -782,7 +782,6 @@ import-linter==2.0 # via -r requirements/edx/testing.in importlib-metadata==6.11.0 # via - # -c requirements/edx/../common_constraints.txt # -r requirements/edx/base.txt # markdown # pytest-randomly diff --git a/requirements/pip-tools.txt b/requirements/pip-tools.txt index 4b631a73d780..d95112e36170 100644 --- a/requirements/pip-tools.txt +++ b/requirements/pip-tools.txt @@ -11,9 +11,7 @@ click==8.1.6 # -c requirements/constraints.txt # pip-tools importlib-metadata==6.11.0 - # via - # -c requirements/common_constraints.txt - # build + # via build packaging==24.0 # via build pip-tools==7.4.1