Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Sync opencraft-release/redwood.1 with Upstream 20241028-1730074895 #704

Open
wants to merge 23 commits into
base: opencraft-release/redwood.1
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
aa70fea
feat: hide the survey report banner for a month after clicking the di…
Asespinel Jun 19, 2024
e5f074b
chore: upgrade Django to 4.2.14
magajh Jul 15, 2024
932e504
Merge pull request #35120 from magajh/open-release/redwood.master
Jul 15, 2024
4744ea1
chore!: uprgade social-auth-app-django to version 5.4.1
iamsobanjaved Jun 10, 2024
670772b
chore: add migration from social_django
iamsobanjaved Jun 27, 2024
2165da0
chore: Re-compile requirements.
feanil Jul 23, 2024
b4a1e01
Merge pull request #35166 from openedx/feanil/backport_django_social_…
Jul 23, 2024
ed72248
fix: libraries across orgs
connorhaugh Feb 15, 2024
d23b41e
docs: imporved comment
connorhaugh Feb 15, 2024
d757cfa
fix: cohorts data can be private
connorhaugh Oct 4, 2023
62269f8
fix: Remove errant pluses from a bad merge.
feanil Jul 25, 2024
8813e8b
style: Fix a pylint and style violations.
feanil Jul 25, 2024
3160ff6
Merge pull request #35180 from openedx/feanil/backporting
Jul 25, 2024
1e0d575
fix: course progress url returned based on course_home_mfe_progress_t…
FatemeKhodayari Jul 30, 2024
c5d7507
chore: upgrade Django to 4.2.15 (#35240)
magajh Aug 6, 2024
d5c84e9
backport fix: disable submit button for archived courses (#34920) to …
Anas12091101 Aug 8, 2024
fa97f13
fix: Prevent error page recursion (#35209)
timmc-edx Jul 31, 2024
3d50dd8
Merge pull request #35259 from raccoongang/max/backport-error-page-re…
cmltaWt0 Aug 8, 2024
5ea3b98
feat: course about page markup and styles improvements (#34892)
ihor-romaniuk Sep 9, 2024
9c8059b
fix: backport: hide new library button for ineligible users in split …
kaustavb12 Sep 19, 2024
9ef785a
chore: upgrade Django to 4.2.16
magajh Sep 10, 2024
daf696a
chore: compile requirements
magajh Oct 9, 2024
5071f28
Merge pull request #35458 from magajh/magajh/patch-django-redwood-4.2.16
cmltaWt0 Oct 17, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 5 additions & 3 deletions cms/djangoapps/contentstore/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -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 []
Expand All @@ -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),
Expand Down Expand Up @@ -1670,7 +1671,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 = []
Expand Down Expand Up @@ -1699,7 +1700,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_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),
Expand Down
56 changes: 45 additions & 11 deletions cms/djangoapps/contentstore/views/library.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -69,22 +69,44 @@ def should_redirect_to_library_authoring_mfe():
)


def user_can_create_library(user, org=None):
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.
"""

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 not LIBRARIES_ENABLED:
return False
elif user.is_staff:
return True
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 = 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()

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_filter_params)
.exists()
)
has_course_admin_role = (
UserBasedRole(user=user, role=CourseInstructorRole.ROLE)
.courses_with_role()
.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.
Expand All @@ -96,6 +118,22 @@ def user_can_create_library(user, org=None):
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'))
Expand All @@ -108,12 +146,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:
Expand Down
39 changes: 25 additions & 14 deletions cms/djangoapps/contentstore/views/tests/test_library.py
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand All @@ -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)
Expand All @@ -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)
Expand Down Expand Up @@ -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)

Expand Down
11 changes: 10 additions & 1 deletion common/djangoapps/util/file.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand All @@ -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.
Expand All @@ -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
Expand Down Expand Up @@ -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:
Expand Down
10 changes: 8 additions & 2 deletions lms/djangoapps/courseware/tests/test_about.py
Original file line number Diff line number Diff line change
Expand Up @@ -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 '<span class="important-dates-item-text pre-requisite"><a href="{}">{}</a></span>'.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 <a href="{pre_requisite_course_about_url}">'
f'{pre_requisite_courses[0]["display"]}</a> 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):
Expand Down Expand Up @@ -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 '<span class="important-dates-item-text pre-requisite"><a href="{}">{}</a></span>'.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 <a href="{pre_requisite_course_about_url}">'
f'{pre_requisite_courses[0]["display"]}</a> 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)
Expand Down
3 changes: 2 additions & 1 deletion lms/djangoapps/instructor/views/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
3 changes: 2 additions & 1 deletion lms/djangoapps/learner_home/serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down Expand Up @@ -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])
Expand Down
26 changes: 25 additions & 1 deletion lms/djangoapps/learner_home/test_serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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):
Expand Down
Loading