diff --git a/.gitignore b/.gitignore index d6f1202bf4ce..1866979edc93 100644 --- a/.gitignore +++ b/.gitignore @@ -136,6 +136,7 @@ build \#*\# .env/ openedx/core/djangoapps/django_comment_common/comment_client/python +openedx/core/djangoapps/cache_toolbox/__pycache__ autodeploy.properties .ws_migrations_complete dist diff --git a/lms/djangoapps/courseware/tests/test_views.py b/lms/djangoapps/courseware/tests/test_views.py index c7d7ce7b177e..e71f3f07ec0d 100644 --- a/lms/djangoapps/courseware/tests/test_views.py +++ b/lms/djangoapps/courseware/tests/test_views.py @@ -30,6 +30,7 @@ from pytz import UTC from openedx.core.djangoapps.waffle_utils.models import WaffleFlagCourseOverrideModel from rest_framework import status +from rest_framework.test import APIClient from web_fragments.fragment import Fragment from xblock.core import XBlock from xblock.fields import Scope, String @@ -76,7 +77,10 @@ from lms.djangoapps.courseware.tests.factories import StudentModuleFactory from lms.djangoapps.courseware.tests.helpers import MasqueradeMixin, get_expiration_banner_text, set_preview_mode from lms.djangoapps.courseware.testutils import RenderXBlockTestMixin -from lms.djangoapps.courseware.toggles import COURSEWARE_OPTIMIZED_RENDER_XBLOCK +from lms.djangoapps.courseware.toggles import ( + COURSEWARE_MICROFRONTEND_SEARCH_ENABLED, + COURSEWARE_OPTIMIZED_RENDER_XBLOCK, +) from lms.djangoapps.courseware.user_state_client import DjangoXBlockUserStateClient from lms.djangoapps.courseware.views.views import ( BasePublicVideoXBlockView, @@ -3683,3 +3687,41 @@ def test_get_template_and_context(self): assert template == 'public_video_share_embed.html' assert context['fragment'] == fragment assert context['course'] == self.course + + +class TestCoursewareMFESearchAPI(SharedModuleStoreTestCase): + """ + Tests the endpoint to fetch the Courseware Search waffle flag enabled status. + """ + + def setUp(self): + super().setUp() + + self.course = CourseFactory.create() + + self.client = APIClient() + self.apiUrl = reverse('courseware_search_enabled_view', kwargs={'course_id': str(self.course.id)}) + + @override_waffle_flag(COURSEWARE_MICROFRONTEND_SEARCH_ENABLED, active=True) + def test_courseware_mfe_search_enabled(self): + """ + Getter to check if user is allowed to use Courseware Search. + """ + + response = self.client.get(self.apiUrl, content_type='application/json') + body = json.loads(response.content.decode('utf-8')) + + self.assertEqual(response.status_code, 200) + self.assertEqual(body, {'enabled': True}) + + @override_waffle_flag(COURSEWARE_MICROFRONTEND_SEARCH_ENABLED, active=False) + def test_is_mfe_search_disabled(self): + """ + Getter to check if user is allowed to use Courseware Search. + """ + + response = self.client.get(self.apiUrl, content_type='application/json') + body = json.loads(response.content.decode('utf-8')) + + self.assertEqual(response.status_code, 200) + self.assertEqual(body, {'enabled': False}) diff --git a/lms/djangoapps/courseware/toggles.py b/lms/djangoapps/courseware/toggles.py index 3e1375512323..78cecac7f584 100644 --- a/lms/djangoapps/courseware/toggles.py +++ b/lms/djangoapps/courseware/toggles.py @@ -55,6 +55,19 @@ f'{WAFFLE_FLAG_NAMESPACE}.mfe_progress_milestones_streak_celebration', __name__ ) +# .. toggle_name: courseware.mfe_courseware_search +# .. toggle_implementation: WaffleFlag +# .. toggle_default: False +# .. toggle_description: Enables Courseware Search on Learning MFE +# .. toggle_use_cases: temporary +# .. toggle_creation_date: 2023-09-28 +# .. toggle_target_removal_date: None +# .. toggle_tickets: KBK-20 +# .. toggle_warning: None. +COURSEWARE_MICROFRONTEND_SEARCH_ENABLED = CourseWaffleFlag( + f'{WAFFLE_FLAG_NAMESPACE}.mfe_courseware_search', __name__ +) + # .. toggle_name: courseware.mfe_progress_milestones_streak_discount_enabled # .. toggle_implementation: CourseWaffleFlag # .. toggle_default: False @@ -153,3 +166,10 @@ def course_is_invitation_only(courselike) -> bool: def learning_assistant_is_active(course_key): return COURSEWARE_LEARNING_ASSISTANT.is_enabled(course_key) + + +def courseware_mfe_search_is_enabled(course_key=None): + """ + Return whether the courseware.mfe_courseware_search flag is on. + """ + return COURSEWARE_MICROFRONTEND_SEARCH_ENABLED.is_enabled(course_key) diff --git a/lms/djangoapps/courseware/views/views.py b/lms/djangoapps/courseware/views/views.py index 085e9e5a19f4..fdc90a46a513 100644 --- a/lms/djangoapps/courseware/views/views.py +++ b/lms/djangoapps/courseware/views/views.py @@ -18,8 +18,8 @@ from django.core.exceptions import PermissionDenied from django.db import transaction from django.db.models import Q, prefetch_related_objects -from django.http import Http404, HttpResponse, HttpResponseBadRequest, HttpResponseForbidden from django.shortcuts import redirect +from django.http import JsonResponse, Http404, HttpResponse, HttpResponseBadRequest, HttpResponseForbidden from django.template.context_processors import csrf from django.urls import reverse from django.utils.decorators import method_decorator @@ -38,8 +38,8 @@ from opaque_keys import InvalidKeyError from opaque_keys.edx.keys import CourseKey, UsageKey from openedx_filters.learning.filters import CourseAboutRenderStarted -from pytz import UTC from requests.exceptions import ConnectionError, Timeout # pylint: disable=redefined-builtin +from pytz import UTC from rest_framework import status from rest_framework.decorators import api_view, throttle_classes from rest_framework.response import Response @@ -87,7 +87,7 @@ from lms.djangoapps.courseware.model_data import FieldDataCache from lms.djangoapps.courseware.models import BaseStudentModuleHistory, StudentModule from lms.djangoapps.courseware.permissions import MASQUERADE_AS_STUDENT, VIEW_COURSE_HOME, VIEW_COURSEWARE -from lms.djangoapps.courseware.toggles import course_is_invitation_only +from lms.djangoapps.courseware.toggles import course_is_invitation_only, courseware_mfe_search_is_enabled from lms.djangoapps.courseware.user_state_client import DjangoXBlockUserStateClient from lms.djangoapps.courseware.utils import ( _use_new_financial_assistance_flow, @@ -2275,3 +2275,15 @@ def get_learner_username(learner_identifier): learner = User.objects.filter(Q(username=learner_identifier) | Q(email=learner_identifier)).first() if learner: return learner.username + + +@api_view(['GET']) +def courseware_mfe_search_enabled(request, course_id=None): + """ + Simple GET endpoint to expose whether the course may use Courseware Search. + """ + + course_key = CourseKey.from_string(course_id) if course_id else None + + payload = {"enabled": courseware_mfe_search_is_enabled(course_key)} + return JsonResponse(payload) diff --git a/lms/urls.py b/lms/urls.py index dc673054df9b..8b314f9f1ea2 100644 --- a/lms/urls.py +++ b/lms/urls.py @@ -752,6 +752,16 @@ ), ] +urlpatterns += [ + re_path( + r'^courses/{}/courseware-search/enabled/$'.format( + settings.COURSE_ID_PATTERN, + ), + courseware_views.courseware_mfe_search_enabled, + name='courseware_search_enabled_view', + ), +] + urlpatterns += [ re_path( r'^courses/{}/lti_tab/(?P[^/]+)/$'.format( diff --git a/openedx/core/djangoapps/agreements/api.py b/openedx/core/djangoapps/agreements/api.py index 7b95607de609..2489cefb8533 100644 --- a/openedx/core/djangoapps/agreements/api.py +++ b/openedx/core/djangoapps/agreements/api.py @@ -9,6 +9,11 @@ from opaque_keys.edx.keys import CourseKey from openedx.core.djangoapps.agreements.models import IntegritySignature +from openedx.core.djangoapps.agreements.models import LTIPIITool +from openedx.core.djangoapps.agreements.models import LTIPIISignature + +from .data import LTIToolsReceivingPIIData +from .data import LTIPIISignatureData log = logging.getLogger(__name__) User = get_user_model() @@ -68,3 +73,170 @@ def get_integrity_signatures_for_course(course_id): """ course_key = CourseKey.from_string(course_id) return IntegritySignature.objects.filter(course_key=course_key) + + +def create_lti_pii_signature(username, course_id, lti_tools): + """ + Creates an lti pii tool signature. If the signature already exist, do not create a new one. + + Arguments: + * course_key (str) + * lti_tools (dict) + * lti_tools_hash (int) + Returns: + * An LTIPIISignature, or None if a signature already exists. + """ + course_key = CourseKey.from_string(course_id) + lti_tools_hash = hash(str(lti_tools)) + + # if user and course exists, update, otherwise create a new signature + try: + user = User.objects.get(username=username) + LTIPIISignature.objects.get(user=user, course_key=course_key) + except User.DoesNotExist: + return None + except LTIPIISignature.DoesNotExist: + signature = LTIPIISignature.objects.create( + user=user, + course_key=course_key, + lti_tools=lti_tools, + lti_tools_hash=lti_tools_hash) + else: + signature = LTIPIISignature.objects.update( + user=user, + course_key=course_key, + lti_tools=lti_tools, + lti_tools_hash=lti_tools_hash) + + return signature + + +def get_lti_pii_signature(username, course_id): + """ + Get the lti pii signature of a user in a course. + + Arguments: + * username (str) + * course_id (str) + + Returns: + * An LTIPIISignature object, or None if one does not exist for the + user + course combination. + """ + course_key = CourseKey.from_string(course_id) + try: + user = User.objects.get(username=username) + signature = LTIPIISignature.objects.get(user=user, course_key=course_key) + except (User.DoesNotExist, LTIPIISignature.DoesNotExist): + return None + else: + return LTIPIISignatureData(user=signature.user, course_id=str(signature.course_key), + lti_tools=signature.lti_tools, lti_tools_hash=signature.lti_tools_hash) + + +def get_pii_receiving_lti_tools(course_id): + """ + Get a course's LTI tools that share PII. + + Arguments: + * course_id (str) + + Returns: + * A List of LTI tools sharing PII. + """ + + course_key = CourseKey.from_string(course_id) + try: + course_ltipiitools = LTIPIITool.objects.get(course_key=course_key).lti_tools + except LTIPIITool.DoesNotExist: + return None + + return LTIToolsReceivingPIIData(lii_tools_receiving_pii=course_ltipiitools) + + +def user_lti_pii_signature_needed(username, course_id): + """ + Determines if a user needs to acknowledge the LTI PII Agreement. + + Arguments: + * username (str) + + Returns: + * True if the user needs to sign a new acknowledgement. + * False if the acknowledgements are up to date. + """ + course_has_lti_pii_tools = _course_has_lti_pii_tools(course_id) + signature_exists = _user_lti_pii_signature_exists(username, course_id) + signature_out_of_date = _user_signature_out_of_date(username, course_id) + + return ((course_has_lti_pii_tools and (not signature_exists)) or + (course_has_lti_pii_tools and signature_exists and signature_out_of_date)) + + +def _course_has_lti_pii_tools(course_id): + """ + Determines if a specifc course has lti tools sharing pii. + + Arguments: + * course_id (str) + + Returns: + * True if the course does have a list. + * False if the course does not. + """ + course_key = CourseKey.from_string(course_id) + try: + course_lti_pii_tools = LTIPIITool.objects.get(course_key=course_key) + except LTIPIITool.DoesNotExist: + # no entry in the database + return False + else: + # returns True if there are entries, and False if the list is empty + return bool(course_lti_pii_tools.lti_tools) + + +def _user_lti_pii_signature_exists(username, course_id): + """ + Determines if a user's lti pii signature exists for a specfic course + + Arguments: + * username (str) + * course_id (str) + + Returns: + * True if user has a signature for the given course. + * False if the user does not have a signature for the given course. + """ + course_key = CourseKey.from_string(course_id) + + try: + user = User.objects.get(username=username) + LTIPIISignature.objects.get(user=user, course_key=course_key) + except (User.DoesNotExist, LTIPIISignature.DoesNotExist): + return False + else: + return True # signature exist + + +def _user_signature_out_of_date(username, course_id): + """ + Determines if a user's existing lti pii signature is out-of-date for a given course. + + Arguments: + * username (str) + * course_id (str) + + Returns: + * True if signature is out-of-date and needs a new signature. + * False if the user has an up-to-date signature. + """ + course_key = CourseKey.from_string(course_id) + + try: + user = User.objects.get(username=username) + user_lti_pii_signature_hash = LTIPIISignature.objects.get(course_key=course_key, user=user).lti_tools_hash + course_lti_pii_tools_hash = LTIPIITool.objects.get(course_key=course_key).lti_tools_hash + except (User.DoesNotExist, LTIPIISignature.DoesNotExist, LTIPIITool.DoesNotExist): + return False + else: + return user_lti_pii_signature_hash != course_lti_pii_tools_hash diff --git a/openedx/core/djangoapps/agreements/data.py b/openedx/core/djangoapps/agreements/data.py new file mode 100644 index 000000000000..9d843c73cb04 --- /dev/null +++ b/openedx/core/djangoapps/agreements/data.py @@ -0,0 +1,23 @@ +""" +Public data structures for this app. +""" +import attr + + +@attr.s(frozen=True, auto_attribs=True) +class LTIToolsReceivingPIIData: + """ + Class that stores data about the list of LTI tools sharing PII + """ + lii_tools_receiving_pii: {} + + +@attr.s(frozen=True, auto_attribs=True) +class LTIPIISignatureData: + """ + Class that stores an lti pii signature + """ + user: str + course_id: str + lti_tools: str + lti_tools_hash: str diff --git a/openedx/core/djangoapps/agreements/tests/test_api.py b/openedx/core/djangoapps/agreements/tests/test_api.py index 45b48f7b4c28..c66065789939 100644 --- a/openedx/core/djangoapps/agreements/tests/test_api.py +++ b/openedx/core/djangoapps/agreements/tests/test_api.py @@ -10,10 +10,17 @@ create_integrity_signature, get_integrity_signature, get_integrity_signatures_for_course, + get_pii_receiving_lti_tools, + create_lti_pii_signature, + get_lti_pii_signature ) from openedx.core.djangolib.testing.utils import skip_unless_lms from xmodule.modulestore.tests.django_utils import SharedModuleStoreTestCase # lint-amnesty, pylint: disable=wrong-import-order from xmodule.modulestore.tests.factories import CourseFactory # lint-amnesty, pylint: disable=wrong-import-order +from ..models import ( + LTIPIITool, +) +from opaque_keys.edx.keys import CourseKey LOGGER_NAME = "openedx.core.djangoapps.agreements.api" @@ -98,3 +105,84 @@ def _assert_integrity_signature(self, signature): """ self.assertEqual(signature.user, self.user) self.assertEqual(signature.course_key, self.course.id) + + +@skip_unless_lms +class TestLTIPIISignatureApi(SharedModuleStoreTestCase): + """ + Tests for the lti pii signature API + """ + @classmethod + def setUpClass(cls): + super().setUpClass() + cls.user = UserFactory() + cls.course = CourseFactory() + cls.course_id = str(cls.course.id) + cls.lti_tools = {"first_lti_tool": "This is the first tool", + "second_lti_tool": "This is the second tool", } + cls.lti_tools_2 = {"first_lti_tool": "This is the first lti tool", + "second_lti_tool": "This is the second tool", + "third_lti_tool": "This is the third tool", } + LTIPIITool.objects.create( + course_key=CourseKey.from_string(cls.course_id), + lti_tools=cls.lti_tools, + lti_tools_hash=11111, + ) + + def test_create_lti_pii_signature(self): + """ + Test to check if an lti pii signature is created from a course and its lti tools. + """ + signature = create_lti_pii_signature(self.user.username, self.course_id, self.lti_tools) + self._assert_lti_pii_signature(signature) + + def test_create_multiple_lti_pii_signature(self): + """ + Test that lti pii signatures are either created or updated + """ + + create_lti_pii_signature(self.user.username, self.course_id, self.lti_tools) # first signature + s1 = get_lti_pii_signature(self.user.username, self.course_id) # retrieve the database entry + create_lti_pii_signature(self.user.username, self.course_id, self.lti_tools_2) # signature with updated tools + s2 = get_lti_pii_signature(self.user.username, self.course_id) # retrieve the updated database entry + self.assertNotEqual(s1, s2) # the signatue retrieved from the database should be the updated version + + def _assert_lti_pii_signature(self, signature): + """ + Helper function to assert the returned lti pii signature has the correct + user and course key + """ + self.assertEqual(signature.user, self.user) + self.assertEqual(signature.course_key, self.course.id) + + +@skip_unless_lms +class TestLTIPIIToolsApi(SharedModuleStoreTestCase): + """ + Tests for the lti pii tool sharing API. To make sure the list of LTI tools can be retreived from the Model. + """ + @classmethod + def setUpClass(cls): + super().setUpClass() + cls.course = CourseFactory() + cls.course_id = str(cls.course.id) + cls.lti_tools = {"first_lti_tool": "This is the first tool", + "second_lti_tool": "This is the second tool", } + LTIPIITool.objects.create( + course_key=CourseKey.from_string(cls.course_id), + lti_tools=cls.lti_tools, + lti_tools_hash=11111, + ) + + def test_get_pii_receiving_lti_tools(self): + """ + Test to check if a course's lti pii tools can be retrieved. + """ + data = get_pii_receiving_lti_tools(self.course_id) + self._assert_ltitools(data.lii_tools_receiving_pii) + + def _assert_ltitools(self, lti_list): + """ + Helper function to assert the returned list has the correct tools + """ + self.assertEqual(self.lti_tools, lti_list) diff --git a/scripts/copy-node-modules.sh b/scripts/copy-node-modules.sh index db997da95785..f49514b6def1 100755 --- a/scripts/copy-node-modules.sh +++ b/scripts/copy-node-modules.sh @@ -26,28 +26,33 @@ log ( ) { echo -e "${COL_LOG}$* $COL_OFF" } +# Print a command (prefixed with '+') and then run it, to aid in debugging. +# This is just like `set -x`, except that `set -x` prints to STDERR, which is hidden +# by GitHub Actions logs. This functions prints to STDOUT, which is visible. +log_and_run ( ) { + log "+$*" + "$@" # Joins arguments to form a command (quoting as necessary) and runs the command. +} + log "=====================================================================================" log "Copying required assets from node_modules..." log "-------------------------------------------------------------------------------" -# Start echoing all commands back to user for ease of debugging. -set -x - log "Ensuring vendor directories exist..." -mkdir -p "$vendor_js" -mkdir -p "$vendor_css" +log_and_run mkdir -p "$vendor_js" +log_and_run mkdir -p "$vendor_css" log "Copying studio-frontend JS & CSS from node_modules into vendor directores..." while read -r -d $'\0' src_file ; do if [[ "$src_file" = *.css ]] || [[ "$src_file" = *.css.map ]] ; then - cp --force "$src_file" "$vendor_css" + log_and_run cp --force "$src_file" "$vendor_css" else - cp --force "$src_file" "$vendor_js" + log_and_run cp --force "$src_file" "$vendor_js" fi done < <(find "$node_modules/@edx/studio-frontend/dist" -type f -print0) log "Copying certain JS modules from node_modules into vendor directory..." -cp --force \ +log_and_run cp --force \ "$node_modules/backbone.paginator/lib/backbone.paginator.js" \ "$node_modules/backbone/backbone.js" \ "$node_modules/bootstrap/dist/js/bootstrap.bundle.js" \ @@ -66,8 +71,8 @@ cp --force \ log "Copying certain JS developer modules into vendor directory..." if [[ "${NODE_ENV:-production}" = development ]] ; then - cp --force "$node_modules/sinon/pkg/sinon.js" "$vendor_js" - cp --force "$node_modules/squirejs/src/Squire.js" "$vendor_js" + log_and_run cp --force "$node_modules/sinon/pkg/sinon.js" "$vendor_js" + log_and_run cp --force "$node_modules/squirejs/src/Squire.js" "$vendor_js" else # TODO: https://github.com/openedx/edx-platform/issues/31768 # In the old implementation of this scipt (pavelib/assets.py), these two @@ -77,13 +82,10 @@ else # However, in the future, it would be good to only copy them for dev # builds. Furthermore, these libraries should not be `npm install`ed # into prod builds in the first place. - cp --force "$node_modules/sinon/pkg/sinon.js" "$vendor_js" || true # "|| true" means "tolerate errors"; in this case, - cp --force "$node_modules/squirejs/src/Squire.js" "$vendor_js" || true # that's "tolerate if these files don't exist." + log_and_run cp --force "$node_modules/sinon/pkg/sinon.js" "$vendor_js" || true # "|| true" means "tolerate errors"; in this case, + log_and_run cp --force "$node_modules/squirejs/src/Squire.js" "$vendor_js" || true # that's "tolerate if these files don't exist." fi -# Done echoing. -set +x - log "-------------------------------------------------------------------------------" log " Done copying required assets from node_modules." log "====================================================================================="