diff --git a/.github/workflows/check-consistent-dependencies.yml b/.github/workflows/check-consistent-dependencies.yml index c2fc5fd1d5..b2466b8830 100644 --- a/.github/workflows/check-consistent-dependencies.yml +++ b/.github/workflows/check-consistent-dependencies.yml @@ -7,13 +7,6 @@ name: Consistent Python dependencies on: pull_request: - paths: - - 'requirements/**' - push: - branches: - - master - paths: - - 'requirements/**' defaults: run: @@ -25,16 +18,37 @@ jobs: runs-on: ubuntu-22.04 steps: + # Only run remaining steps if there are changes to requirements/** + - name: "Decide whether to short-circuit" + env: + GH_TOKEN: "${{ github.token }}" + PR_URL: "${{ github.event.pull_request.html_url }}" + run: | + paths=$(gh pr diff "$PR_URL" --name-only) + echo $'Paths touched in PR:\n'"$paths" + + # The ^"? is because git may quote weird file paths + matched="$(echo "$paths" | grep -P '^"?requirements/' || true)" + echo $'Relevant paths:\n'"$matched" + if [[ -n "$matched" ]]; then + echo "RELEVANT=true" >> "$GITHUB_ENV" + fi + - uses: actions/checkout@v3 + if: ${{ env.RELEVANT == 'true' }} - uses: actions/setup-python@v4 + if: ${{ env.RELEVANT == 'true' }} with: python-version: '3.8' - - run: | + - name: "Recompile requirements" + if: ${{ env.RELEVANT == 'true' }} + run: | make compile-requirements - name: Fail if compiling requirements caused changes + if: ${{ env.RELEVANT == 'true' }} run: | SUMMARY_HELP=$(cat <<'EOMARKDOWN' # Inconsistent Python dependencies diff --git a/cms/djangoapps/contentstore/asset_storage_handlers.py b/cms/djangoapps/contentstore/asset_storage_handlers.py index 29c8800fa0..8808dc4f0b 100644 --- a/cms/djangoapps/contentstore/asset_storage_handlers.py +++ b/cms/djangoapps/contentstore/asset_storage_handlers.py @@ -32,7 +32,7 @@ from xmodule.modulestore.django import modulestore # lint-amnesty, pylint: disa from xmodule.modulestore.exceptions import ItemNotFoundError # lint-amnesty, pylint: disable=wrong-import-order from .exceptions import AssetNotFoundException, AssetSizeTooLargeException -from .utils import reverse_course_url, get_files_uploads_url +from .utils import reverse_course_url, get_files_uploads_url, get_response_format, request_response_format_is_json from .toggles import use_new_files_uploads_page @@ -73,8 +73,8 @@ def handle_assets(request, course_key_string=None, asset_key_string=None): if not has_course_author_access(request.user, course_key): raise PermissionDenied() - response_format = _get_response_format(request) - if _request_response_format_is_json(request, response_format): + response_format = get_response_format(request) + if request_response_format_is_json(request, response_format): if request.method == 'GET': return _assets_json(request, course_key) @@ -133,14 +133,6 @@ def get_asset_usage_path(request, course_key, asset_key_string): return JsonResponse({'usage_locations': usage_locations}) -def _get_response_format(request): - return request.GET.get('format') or request.POST.get('format') or 'html' - - -def _request_response_format_is_json(request, response_format): - return response_format == 'json' or 'application/json' in request.META.get('HTTP_ACCEPT', 'application/json') - - def _asset_index(request, course_key): ''' Display an editable asset library. diff --git a/cms/djangoapps/contentstore/tests/test_video_utils.py b/cms/djangoapps/contentstore/tests/test_video_utils.py index 80d55b92b8..c81761a283 100644 --- a/cms/djangoapps/contentstore/tests/test_video_utils.py +++ b/cms/djangoapps/contentstore/tests/test_video_utils.py @@ -5,7 +5,7 @@ Unit tests for video utils. from datetime import datetime from unittest import TestCase -from unittest.mock import MagicMock, patch +from unittest import mock import ddt import pytz @@ -144,7 +144,7 @@ class ScrapeVideoThumbnailsTestCase(CourseTestCase): return mocked_response @override_settings(AWS_ACCESS_KEY_ID='test_key_id', AWS_SECRET_ACCESS_KEY='test_secret') - @patch('requests.get') + @mock.patch('requests.get') @ddt.data( ( { @@ -228,7 +228,7 @@ class ScrapeVideoThumbnailsTestCase(CourseTestCase): self.assertEqual(thumbnail_content_type, 'image/jpeg') @override_settings(AWS_ACCESS_KEY_ID='test_key_id', AWS_SECRET_ACCESS_KEY='test_secret') - @patch('requests.get') + @mock.patch('requests.get') def test_scrape_youtube_thumbnail(self, mocked_request): """ Test that youtube thumbnails are correctly scrapped. @@ -273,8 +273,8 @@ class ScrapeVideoThumbnailsTestCase(CourseTestCase): ) ) @override_settings(AWS_ACCESS_KEY_ID='test_key_id', AWS_SECRET_ACCESS_KEY='test_secret') - @patch('cms.djangoapps.contentstore.video_utils.LOGGER') - @patch('requests.get') + @mock.patch('cms.djangoapps.contentstore.video_utils.LOGGER') + @mock.patch('requests.get') @ddt.unpack def test_scrape_youtube_thumbnail_logging( self, @@ -333,8 +333,8 @@ class ScrapeVideoThumbnailsTestCase(CourseTestCase): ) ), ) - @patch('cms.djangoapps.contentstore.video_utils.LOGGER') - @patch('cms.djangoapps.contentstore.video_utils.download_youtube_video_thumbnail') + @mock.patch('cms.djangoapps.contentstore.video_utils.LOGGER') + @mock.patch('cms.djangoapps.contentstore.video_utils.download_youtube_video_thumbnail') @ddt.unpack def test_no_video_thumbnail_downloaded( self, @@ -376,7 +376,7 @@ class S3Boto3TestCase(TestCase): def setUp(self): self.storage = S3Boto3Storage() - self.storage._connections.connection = MagicMock() # pylint: disable=protected-access + self.storage._connections.connection = mock.MagicMock() # pylint: disable=protected-access def order_dict(self, dictionary): """ @@ -417,18 +417,18 @@ class S3Boto3TestCase(TestCase): content = ContentFile('new content') storage = S3Boto3Storage(**{'bucket_name': 'test'}) - storage._connections.connection = MagicMock() # pylint: disable=protected-access + storage._connections.connection = mock.MagicMock() # pylint: disable=protected-access storage.save(name, content) storage.bucket.Object.assert_called_once_with(name) obj = storage.bucket.Object.return_value obj.upload_fileobj.assert_called_with( - content, + mock.ANY, ExtraArgs=self.order_dict({ 'ContentType': 'text/plain', }), - Config=storage._transfer_config # pylint: disable=protected-access + Config=storage.transfer_config # pylint: disable=protected-access ) @override_settings(AWS_DEFAULT_ACL='public-read') @@ -445,7 +445,7 @@ class S3Boto3TestCase(TestCase): name = 'test_storage_save.txt' content = ContentFile('new content') storage = S3Boto3Storage(**{'bucket_name': 'test', 'default_acl': default_acl}) - storage._connections.connection = MagicMock() # pylint: disable=protected-access + storage._connections.connection = mock.MagicMock() # pylint: disable=protected-access storage.save(name, content) storage.bucket.Object.assert_called_once_with(name) @@ -461,9 +461,9 @@ class S3Boto3TestCase(TestCase): del ExtraArgs['ACL'] obj.upload_fileobj.assert_called_with( - content, + mock.ANY, ExtraArgs=self.order_dict(ExtraArgs), - Config=storage._transfer_config # pylint: disable=protected-access + Config=storage.transfer_config # pylint: disable=protected-access ) @ddt.data('public-read', 'private') @@ -476,16 +476,16 @@ class S3Boto3TestCase(TestCase): content = ContentFile('new content') storage = S3Boto3Storage(**{'bucket_name': 'test', 'default_acl': None}) - storage._connections.connection = MagicMock() # pylint: disable=protected-access + storage._connections.connection = mock.MagicMock() # pylint: disable=protected-access storage.save(name, content) storage.bucket.Object.assert_called_once_with(name) obj = storage.bucket.Object.return_value obj.upload_fileobj.assert_called_with( - content, - ExtraArgs=self.order_dict({ + mock.ANY, + Config=storage.transfer_config, # pylint: disable=protected-access + ExtraArgs={ 'ContentType': 'text/plain', - }), - Config=storage._transfer_config # pylint: disable=protected-access + }, ) diff --git a/cms/djangoapps/contentstore/utils.py b/cms/djangoapps/contentstore/utils.py index 97b70e828c..1a4b709622 100644 --- a/cms/djangoapps/contentstore/utils.py +++ b/cms/djangoapps/contentstore/utils.py @@ -1390,9 +1390,63 @@ def get_help_urls(): return help_tokens +def get_response_format(request): + return request.GET.get('format') or request.POST.get('format') or 'html' + + +def request_response_format_is_json(request, response_format): + return response_format == 'json' or 'application/json' in request.META.get('HTTP_ACCEPT', 'application/json') + + +def get_library_context(request, request_is_json=False): + """ + Utils is used to get context of course home library tab. + It is used for both DRF and django views. + """ + from cms.djangoapps.contentstore.views.course import ( + get_allowed_organizations, + get_allowed_organizations_for_libraries, + user_can_create_organizations, + _accessible_libraries_iter, + _get_course_creator_status, + _format_library_for_view, + ) + from cms.djangoapps.contentstore.views.library import ( + LIBRARIES_ENABLED, + ) + + libraries = _accessible_libraries_iter(request.user) if LIBRARIES_ENABLED else [] + data = { + 'libraries': [_format_library_for_view(lib, request) for lib in libraries], + } + + if not request_is_json: + return { + **data, + 'in_process_course_actions': [], + 'courses': [], + 'libraries_enabled': LIBRARIES_ENABLED, + 'show_new_library_button': LIBRARIES_ENABLED 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), + 'allow_unicode_course_id': settings.FEATURES.get('ALLOW_UNICODE_COURSE_ID', False), + 'archived_courses': True, + 'allow_course_reruns': settings.FEATURES.get('ALLOW_COURSE_RERUNS', True), + 'rerun_creator_status': GlobalStaff().has_user(request.user), + 'split_studio_home': split_library_view_on_dashboard(), + 'active_tab': 'libraries', + 'allowed_organizations_for_libraries': get_allowed_organizations_for_libraries(request.user), + 'allowed_organizations': get_allowed_organizations(request.user), + 'can_create_organizations': user_can_create_organizations(request.user), + } + + return data + + def get_home_context(request): """ - Utils is used to get context of course grading. + Utils is used to get context of course home. It is used for both DRF and django views. """ @@ -1420,8 +1474,14 @@ def get_home_context(request): courses_iter, in_process_course_actions = get_courses_accessible_to_user(request, org) user = request.user libraries = [] + response_format = get_response_format(request) + if not split_library_view_on_dashboard() and LIBRARIES_ENABLED: - libraries = _accessible_libraries_iter(request.user) + accessible_libraries = _accessible_libraries_iter(user) + libraries = [_format_library_for_view(lib, request) for lib in accessible_libraries] + + if split_library_view_on_dashboard() and request_response_format_is_json(request, response_format): + libraries = get_library_context(request, True)['libraries'] def format_in_process_course_view(uca): """ @@ -1456,7 +1516,7 @@ def get_home_context(request): 'libraries_enabled': LIBRARIES_ENABLED, 'redirect_to_library_authoring_mfe': should_redirect_to_library_authoring_mfe(), 'library_authoring_mfe_url': LIBRARY_AUTHORING_MICROFRONTEND_URL, - 'libraries': [_format_library_for_view(lib, request) for lib in libraries], + 'libraries': libraries, 'show_new_library_button': user_can_create_library(user) and not should_redirect_to_library_authoring_mfe(), 'user': user, 'request_course_creator_url': reverse('request_course_creator'), diff --git a/cms/djangoapps/contentstore/views/course.py b/cms/djangoapps/contentstore/views/course.py index d74a99a8e3..4a70e5028c 100644 --- a/cms/djangoapps/contentstore/views/course.py +++ b/cms/djangoapps/contentstore/views/course.py @@ -89,7 +89,6 @@ from ..courseware_index import CoursewareSearchIndexer, SearchIndexingError from ..tasks import rerun_course as rerun_course_task from ..toggles import ( default_enable_flexible_peer_openassessments, - split_library_view_on_dashboard, use_new_course_outline_page, use_new_home_page, use_new_updates_page, @@ -102,6 +101,7 @@ from ..utils import ( get_course_settings, get_course_grading, get_home_context, + get_library_context, get_lms_link_for_item, get_proctored_exam_settings_url, get_course_outline_url, @@ -121,7 +121,6 @@ from ..utils import ( update_course_discussions_settings, ) from .component import ADVANCED_COMPONENT_TYPES -from .library import LIBRARIES_ENABLED log = logging.getLogger(__name__) User = get_user_model() @@ -551,26 +550,7 @@ def library_listing(request): """ List all Libraries available to the logged in user """ - libraries = _accessible_libraries_iter(request.user) if LIBRARIES_ENABLED else [] - data = { - 'in_process_course_actions': [], - 'courses': [], - 'libraries_enabled': LIBRARIES_ENABLED, - 'libraries': [_format_library_for_view(lib, request) for lib in libraries], - 'show_new_library_button': LIBRARIES_ENABLED 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), - 'allow_unicode_course_id': settings.FEATURES.get('ALLOW_UNICODE_COURSE_ID', False), - 'archived_courses': True, - 'allow_course_reruns': settings.FEATURES.get('ALLOW_COURSE_RERUNS', True), - 'rerun_creator_status': GlobalStaff().has_user(request.user), - 'split_studio_home': split_library_view_on_dashboard(), - 'active_tab': 'libraries', - 'allowed_organizations': get_allowed_organizations(request.user), - 'allowed_organizations_for_libraries': get_allowed_organizations_for_libraries(request.user), - 'can_create_organizations': user_can_create_organizations(request.user), - } + data = get_library_context(request) return render_to_response('index.html', data) diff --git a/cms/djangoapps/contentstore/views/preview.py b/cms/djangoapps/contentstore/views/preview.py index 5ee2740abe..48c323c1aa 100644 --- a/cms/djangoapps/contentstore/views/preview.py +++ b/cms/djangoapps/contentstore/views/preview.py @@ -313,7 +313,8 @@ def _studio_wrap_xblock(xblock, view, frag, context, display_name_only=False): 'selected_groups_label': selected_groups_label, 'can_add': context.get('can_add', True), 'can_move': context.get('can_move', is_course), - 'language': getattr(course, 'language', None) + 'language': getattr(course, 'language', None), + 'is_course': is_course } add_webpack_js_to_fragment(frag, "js/factories/xblock_validation") diff --git a/cms/static/js/views/pages/container.js b/cms/static/js/views/pages/container.js index 122556c76a..8f296ebb6b 100644 --- a/cms/static/js/views/pages/container.js +++ b/cms/static/js/views/pages/container.js @@ -48,6 +48,7 @@ function($, _, Backbone, gettext, BasePage, ViewUtils, ContainerView, XBlockView BasePage.prototype.initialize.call(this, options); this.viewClass = options.viewClass || this.defaultViewClass; this.isLibraryPage = (this.model.attributes.category === 'library'); + this.isLibraryContentPage = (this.model.attributes.category === 'library_content'); this.nameEditor = new XBlockStringFieldEditor({ el: this.$('.wrapper-xblock-field'), model: this.model @@ -154,7 +155,7 @@ function($, _, Backbone, gettext, BasePage, ViewUtils, ContainerView, XBlockView self.delegateEvents(); // Show/hide the paste button - if (!self.isLibraryPage) { + if (!self.isLibraryPage && !self.isLibraryContentPage) { self.initializePasteButton(); } }, @@ -208,32 +209,36 @@ function($, _, Backbone, gettext, BasePage, ViewUtils, ContainerView, XBlockView * Given the latest information about the user's clipboard, hide or show the Paste button as appropriate. */ refreshPasteButton(data) { - // 'data' is the same data returned by the "get clipboard status" API endpoint - // i.e. /api/content-staging/v1/clipboard/ - if (this.options.canEdit && data.content) { - if (["vertical", "sequential", "chapter", "course"].includes(data.content.block_type)) { - // This is not suitable for pasting into a unit. - this.$(".paste-component").hide(); - } else if (data.content.status === "expired") { - // This has expired and can no longer be pasted. - this.$(".paste-component").hide(); - } else { - // The thing in the clipboard can be pasted into this unit: - const detailsPopupEl = this.$(".clipboard-details-popup")[0]; - detailsPopupEl.querySelector(".detail-block-name").innerText = data.content.display_name; - detailsPopupEl.querySelector(".detail-block-type").innerText = data.content.block_type_display; - detailsPopupEl.querySelector(".detail-course-name").innerText = data.source_context_title; - if (data.source_edit_url) { - detailsPopupEl.setAttribute("href", data.source_edit_url); - detailsPopupEl.classList.remove("no-edit-link"); + // Do not perform any changes on paste button since they are not + // rendered on Library or LibraryContent pages + if (!this.isLibraryPage && !this.isLibraryContentPage) { + // 'data' is the same data returned by the "get clipboard status" API endpoint + // i.e. /api/content-staging/v1/clipboard/ + if (this.options.canEdit && data.content) { + if (["vertical", "sequential", "chapter", "course"].includes(data.content.block_type)) { + // This is not suitable for pasting into a unit. + this.$(".paste-component").hide(); + } else if (data.content.status === "expired") { + // This has expired and can no longer be pasted. + this.$(".paste-component").hide(); } else { - detailsPopupEl.setAttribute("href", "#"); - detailsPopupEl.classList.add("no-edit-link"); + // The thing in the clipboard can be pasted into this unit: + const detailsPopupEl = this.$(".clipboard-details-popup")[0]; + detailsPopupEl.querySelector(".detail-block-name").innerText = data.content.display_name; + detailsPopupEl.querySelector(".detail-block-type").innerText = data.content.block_type_display; + detailsPopupEl.querySelector(".detail-course-name").innerText = data.source_context_title; + if (data.source_edit_url) { + detailsPopupEl.setAttribute("href", data.source_edit_url); + detailsPopupEl.classList.remove("no-edit-link"); + } else { + detailsPopupEl.setAttribute("href", "#"); + detailsPopupEl.classList.add("no-edit-link"); + } + this.$(".paste-component").show(); } - this.$(".paste-component").show(); + } else { + this.$(".paste-component").hide(); } - } else { - this.$(".paste-component").hide(); } }, diff --git a/cms/templates/studio_xblock_wrapper.html b/cms/templates/studio_xblock_wrapper.html index fa48a905cc..a027cc764b 100644 --- a/cms/templates/studio_xblock_wrapper.html +++ b/cms/templates/studio_xblock_wrapper.html @@ -104,9 +104,15 @@ block_is_unit = is_unit(xblock)