From 447ee973e7e64ce8fe8f0966b5ee763ee6d1d3e8 Mon Sep 17 00:00:00 2001 From: Sandeep Kumar Choudhary Date: Mon, 7 Jun 2021 05:52:39 +0000 Subject: [PATCH] feat: Allow delete course content in Studio only for admin users --- cms/djangoapps/contentstore/config/waffle.py | 10 ++ cms/djangoapps/contentstore/permissions.py | 10 ++ .../contentstore/views/tests/test_block.py | 147 ++++++++++++++++++ .../xblock_storage_handlers/view_handlers.py | 9 +- .../spec/views/pages/course_outline_spec.js | 20 ++- cms/static/js/views/xblock_outline.js | 1 + cms/templates/js/course-outline.underscore | 4 +- setup.cfg | 1 + 8 files changed, 195 insertions(+), 7 deletions(-) create mode 100644 cms/djangoapps/contentstore/permissions.py diff --git a/cms/djangoapps/contentstore/config/waffle.py b/cms/djangoapps/contentstore/config/waffle.py index 8f431fa49327..16719e61bf29 100644 --- a/cms/djangoapps/contentstore/config/waffle.py +++ b/cms/djangoapps/contentstore/config/waffle.py @@ -53,3 +53,13 @@ # .. toggle_warning: Flag course_experience.relative_dates should also be active for relative dates functionalities to work. # .. toggle_tickets: https://openedx.atlassian.net/browse/AA-844 CUSTOM_RELATIVE_DATES = CourseWaffleFlag(f'{WAFFLE_NAMESPACE}.custom_relative_dates', __name__) + +# .. toggle_name: studio.prevent_staff_structure_deletion +# .. toggle_implementation: WaffleFlag +# .. toggle_default: False +# .. toggle_description: Prevents staff from deleting course structures +# .. toggle_use_cases: opt_in +# .. toggle_creation_date: 2021-06-25 +PREVENT_STAFF_STRUCTURE_DELETION = WaffleFlag( + f'{WAFFLE_NAMESPACE}.prevent_staff_structure_deletion', __name__, LOG_PREFIX +) diff --git a/cms/djangoapps/contentstore/permissions.py b/cms/djangoapps/contentstore/permissions.py new file mode 100644 index 000000000000..14fe40c09ca7 --- /dev/null +++ b/cms/djangoapps/contentstore/permissions.py @@ -0,0 +1,10 @@ +""" +Permission definitions for the contentstore djangoapp +""" + +from bridgekeeper import perms + +from lms.djangoapps.courseware.rules import HasRolesRule + +DELETE_COURSE_CONTENT = 'contentstore.delete_course_content' +perms[DELETE_COURSE_CONTENT] = HasRolesRule('instructor') diff --git a/cms/djangoapps/contentstore/views/tests/test_block.py b/cms/djangoapps/contentstore/views/tests/test_block.py index a25d0670b4be..678cd01207e1 100644 --- a/cms/djangoapps/contentstore/views/tests/test_block.py +++ b/cms/djangoapps/contentstore/views/tests/test_block.py @@ -17,6 +17,7 @@ from openedx_events.content_authoring.signals import XBLOCK_DUPLICATED from openedx_events.tests.utils import OpenEdxEventsTestMixin from edx_proctoring.exceptions import ProctoredExamNotFoundException +from edx_toggles.toggles.testutils import override_waffle_flag from opaque_keys import InvalidKeyError from opaque_keys.edx.asides import AsideUsageKeyV2 from opaque_keys.edx.keys import CourseKey, UsageKey @@ -63,6 +64,8 @@ update_from_source, ) from cms.djangoapps.contentstore.xblock_storage_handlers import view_handlers as item_module +from cms.djangoapps.contentstore.config.waffle import PREVENT_STAFF_STRUCTURE_DELETION +from common.djangoapps.student.roles import CourseInstructorRole, CourseStaffRole, CourseCreatorRole from common.djangoapps.student.tests.factories import StaffFactory, UserFactory from common.djangoapps.xblock_django.models import ( XBlockConfiguration, @@ -4218,6 +4221,150 @@ def test_self_paced_item_visibility_state(self, store_type): xblock_info = self._get_xblock_info(chapter.location) self._verify_visibility_state(xblock_info, VisibilityState.live) + def test_staff_show_delete_button(self): + """ + Test delete button is *not visible* to user with CourseStaffRole + """ + # Add user as course staff + CourseStaffRole(self.course_key).add_users(self.user) + + # Get xblock outline + xblock_info = create_xblock_info( + self.course, + include_child_info=True, + course_outline=True, + include_children_predicate=lambda xblock: not xblock.category == 'vertical', + user=self.user + ) + self.assertTrue(xblock_info['show_delete_button']) + + def test_staff_show_delete_button_with_waffle(self): + """ + Test delete button is *not visible* to user with CourseStaffRole and + PREVENT_STAFF_STRUCTURE_DELETION waffle set + """ + # Add user as course staff + CourseStaffRole(self.course_key).add_users(self.user) + + with override_waffle_flag(PREVENT_STAFF_STRUCTURE_DELETION, active=True): + # Get xblock outline + xblock_info = create_xblock_info( + self.course, + include_child_info=True, + course_outline=True, + include_children_predicate=lambda xblock: not xblock.category == 'vertical', + user=self.user + ) + + self.assertFalse(xblock_info['show_delete_button']) + + def test_no_user_show_delete_button(self): + """ + Test delete button is *visible* when user attribute is not set on + xblock. This happens with ajax requests. + """ + # Get xblock outline + xblock_info = create_xblock_info( + self.course, + include_child_info=True, + course_outline=True, + include_children_predicate=lambda xblock: not xblock.category == 'vertical', + user=None + ) + self.assertTrue(xblock_info['show_delete_button']) + + def test_no_user_show_delete_button_with_waffle(self): + """ + Test delete button is *visible* when user attribute is not set on + xblock (this happens with ajax requests) and PREVENT_STAFF_STRUCTURE_DELETION waffle set. + """ + + with override_waffle_flag(PREVENT_STAFF_STRUCTURE_DELETION, active=True): + # Get xblock outline + xblock_info = create_xblock_info( + self.course, + include_child_info=True, + course_outline=True, + include_children_predicate=lambda xblock: not xblock.category == 'vertical', + user=None + ) + + self.assertFalse(xblock_info['show_delete_button']) + + def test_instructor_show_delete_button(self): + """ + Test delete button is *visible* to user with CourseInstructorRole only + """ + # Add user as course instructor + CourseInstructorRole(self.course_key).add_users(self.user) + + # Get xblock outline + xblock_info = create_xblock_info( + self.course, + include_child_info=True, + course_outline=True, + include_children_predicate=lambda xblock: not xblock.category == 'vertical', + user=self.user + ) + self.assertTrue(xblock_info['show_delete_button']) + + def test_instructor_show_delete_button_with_waffle(self): + """ + Test delete button is *visible* to user with CourseInstructorRole only + and PREVENT_STAFF_STRUCTURE_DELETION waffle set + """ + # Add user as course instructor + CourseInstructorRole(self.course_key).add_users(self.user) + + with override_waffle_flag(PREVENT_STAFF_STRUCTURE_DELETION, active=True): + # Get xblock outline + xblock_info = create_xblock_info( + self.course, + include_child_info=True, + course_outline=True, + include_children_predicate=lambda xblock: not xblock.category == 'vertical', + user=self.user + ) + + self.assertTrue(xblock_info['show_delete_button']) + + def test_creator_show_delete_button(self): + """ + Test delete button is *visible* to user with CourseInstructorRole only + """ + # Add user as course creator + CourseCreatorRole(self.course_key).add_users(self.user) + + # Get xblock outline + xblock_info = create_xblock_info( + self.course, + include_child_info=True, + course_outline=True, + include_children_predicate=lambda xblock: not xblock.category == 'vertical', + user=self.user + ) + self.assertTrue(xblock_info['show_delete_button']) + + def test_creator_show_delete_button_with_waffle(self): + """ + Test delete button is *visible* to user with CourseInstructorRole only + and PREVENT_STAFF_STRUCTURE_DELETION waffle set + """ + # Add user as course creator + CourseCreatorRole(self.course_key).add_users(self.user) + + with override_waffle_flag(PREVENT_STAFF_STRUCTURE_DELETION, active=True): + # Get xblock outline + xblock_info = create_xblock_info( + self.course, + include_child_info=True, + course_outline=True, + include_children_predicate=lambda xblock: not xblock.category == 'vertical', + user=self.user + ) + + self.assertFalse(xblock_info['show_delete_button']) + @patch( "xmodule.modulestore.split_mongo.caching_descriptor_system.CachingDescriptorSystem.applicable_aside_types", diff --git a/cms/djangoapps/contentstore/xblock_storage_handlers/view_handlers.py b/cms/djangoapps/contentstore/xblock_storage_handlers/view_handlers.py index f7d369630769..16ba419bfbf7 100644 --- a/cms/djangoapps/contentstore/xblock_storage_handlers/view_handlers.py +++ b/cms/djangoapps/contentstore/xblock_storage_handlers/view_handlers.py @@ -37,7 +37,8 @@ from xblock.core import XBlock from xblock.fields import Scope -from cms.djangoapps.contentstore.config.waffle import SHOW_REVIEW_RULES_FLAG +from cms.djangoapps.contentstore.config.waffle import PREVENT_STAFF_STRUCTURE_DELETION, SHOW_REVIEW_RULES_FLAG +from cms.djangoapps.contentstore.permissions import DELETE_COURSE_CONTENT from cms.djangoapps.contentstore.toggles import ENABLE_COPY_PASTE_UNITS from cms.djangoapps.models.settings.course_grading import CourseGradingModel from cms.lib.ai_aside_summary_config import AiAsideSummaryConfig @@ -1404,6 +1405,12 @@ def create_xblock_info( # lint-amnesty, pylint: disable=too-many-statements xblock, course=course ) + xblock_info['show_delete_button'] = True + if PREVENT_STAFF_STRUCTURE_DELETION.is_enabled(): + xblock_info['show_delete_button'] = ( + user.has_perm(DELETE_COURSE_CONTENT, xblock) if user is not None else False + ) + if is_xblock_unit and summary_configuration.is_enabled(): xblock_info["summary_configuration_enabled"] = summary_configuration.is_summary_enabled(xblock_info['id']) diff --git a/cms/static/js/spec/views/pages/course_outline_spec.js b/cms/static/js/spec/views/pages/course_outline_spec.js index a230d17797c7..5905bc51a6c2 100644 --- a/cms/static/js/spec/views/pages/course_outline_spec.js +++ b/cms/static/js/spec/views/pages/course_outline_spec.js @@ -42,6 +42,7 @@ describe('CourseOutlinePage', function() { user_partition_info: {}, highlights_enabled: true, highlights_enabled_for_messaging: false, + show_delete_button: true }, options, {child_info: {children: children}}); }; @@ -68,7 +69,8 @@ describe('CourseOutlinePage', function() { show_review_rules: true, user_partition_info: {}, highlights_enabled: true, - highlights_enabled_for_messaging: false + highlights_enabled_for_messaging: false, + show_delete_button: true }, options, {child_info: {children: children}}); }; @@ -93,7 +95,8 @@ describe('CourseOutlinePage', function() { group_access: {}, user_partition_info: {}, highlights: [], - highlights_enabled: true + highlights_enabled: true, + show_delete_button: true }, options, {child_info: {children: children}}); }; @@ -123,7 +126,8 @@ describe('CourseOutlinePage', function() { }, user_partitions: [], group_access: {}, - user_partition_info: {} + user_partition_info: {}, + show_delete_button: true }, options, {child_info: {children: children}}); }; @@ -141,7 +145,8 @@ describe('CourseOutlinePage', function() { edited_by: 'MockUser', user_partitions: [], group_access: {}, - user_partition_info: {} + user_partition_info: {}, + show_delete_button: true }, options); }; @@ -934,6 +939,13 @@ describe('CourseOutlinePage', function() { expect(outlinePage.$('[data-locator="mock-section-2"]')).toExist(); }); + it('remains un-visible if show_delete_button is false ', function() { + createCourseOutlinePage(this, createMockCourseJSON({show_delete_button: false}, [ + createMockSectionJSON({show_delete_button: false}) + ])); + expect(getItemHeaders('section').find('.delete-button').first()).not.toExist(); + }); + it('can be deleted if it is the only section', function() { var promptSpy = EditHelpers.createPromptSpy(); createCourseOutlinePage(this, mockSingleSectionCourseJSON); diff --git a/cms/static/js/views/xblock_outline.js b/cms/static/js/views/xblock_outline.js index 9ded8d66e91f..2555200b39d8 100644 --- a/cms/static/js/views/xblock_outline.js +++ b/cms/static/js/views/xblock_outline.js @@ -114,6 +114,7 @@ function($, _, gettext, BaseView, ViewUtils, XBlockViewUtils, XBlockStringFieldE staffOnlyMessage: this.model.get('staff_only_message'), course: course, enableCopyPasteUnits: this.model.get("enable_copy_paste_units"), // ENABLE_COPY_PASTE_UNITS waffle flag + showDeleteButton: this.model.get('show_delete_button') }; }, diff --git a/cms/templates/js/course-outline.underscore b/cms/templates/js/course-outline.underscore index fa3d562196b4..0d0756cd447b 100644 --- a/cms/templates/js/course-outline.underscore +++ b/cms/templates/js/course-outline.underscore @@ -198,7 +198,7 @@ if (is_proctored_exam) { <%- gettext('Duplicate') %> <% } %> - <% if (xblockInfo.isDeletable()) { %> + <% if (xblockInfo.isDeletable() && showDeleteButton) { %>
  • <%- gettext('Delete') %>
    • @@ -224,7 +224,7 @@ if (is_proctored_exam) { <% } %> - <% if (xblockInfo.isDeletable()) { %> + <% if (xblockInfo.isDeletable() && showDeleteButton) { %>
  • diff --git a/setup.cfg b/setup.cfg index fe889355a753..8c7b93d35d95 100644 --- a/setup.cfg +++ b/setup.cfg @@ -158,6 +158,7 @@ ignore_imports = # -> openedx.features.enterprise_support.utils openedx.features.enterprise_support.utils -> lms.djangoapps.branding.api cms.djangoapps.contentstore.rest_api.v1.views.settings -> lms.djangoapps.certificates.api + cms.djangoapps.contentstore.permissions -> lms.djangoapps.courseware.rules [importlinter:contract:2]