[BD-14] Limit number of blocks allowed in content libraries (#24276)
* Enforce limit on number of blocks allowed in library (blockstore) * Enforce limit on number of blocks allowed in library (modulestore) * Changes from review feedback
This commit is contained in:
@@ -103,6 +103,19 @@ def _filter_entrance_exam_grader(graders):
|
||||
return graders
|
||||
|
||||
|
||||
def _is_library_component_limit_reached(usage_key):
|
||||
"""
|
||||
Verify if the library has reached the maximum number of components allowed in it
|
||||
"""
|
||||
store = modulestore()
|
||||
parent = store.get_item(usage_key)
|
||||
if not parent.has_children:
|
||||
# Limit cannot be applied on such items
|
||||
return False
|
||||
total_children = len(parent.children)
|
||||
return total_children + 1 > settings.MAX_BLOCKS_PER_CONTENT_LIBRARY
|
||||
|
||||
|
||||
@require_http_methods(("DELETE", "GET", "PUT", "POST", "PATCH"))
|
||||
@login_required
|
||||
@expect_json
|
||||
@@ -215,6 +228,18 @@ def xblock_handler(request, usage_key_string):
|
||||
):
|
||||
raise PermissionDenied()
|
||||
|
||||
# Libraries have a maximum component limit enforced on them
|
||||
if (isinstance(parent_usage_key, LibraryUsageLocator) and
|
||||
_is_library_component_limit_reached(parent_usage_key)):
|
||||
return JsonResponse(
|
||||
{
|
||||
'error': _(u'Libraries cannot have more than {limit} components').format(
|
||||
limit=settings.MAX_BLOCKS_PER_CONTENT_LIBRARY
|
||||
)
|
||||
},
|
||||
status=400
|
||||
)
|
||||
|
||||
dest_usage_key = _duplicate_item(
|
||||
parent_usage_key,
|
||||
duplicate_source_usage_key,
|
||||
@@ -691,6 +716,16 @@ def _create_item(request):
|
||||
u"Category '%s' not supported for Libraries" % category, content_type='text/plain'
|
||||
)
|
||||
|
||||
if _is_library_component_limit_reached(usage_key):
|
||||
return JsonResponse(
|
||||
{
|
||||
'error': _(u'Libraries cannot have more than {limit} components').format(
|
||||
limit=settings.MAX_BLOCKS_PER_CONTENT_LIBRARY
|
||||
)
|
||||
},
|
||||
status=400
|
||||
)
|
||||
|
||||
created_block = create_xblock(
|
||||
parent_locator=parent_locator,
|
||||
user=request.user,
|
||||
|
||||
@@ -8,6 +8,7 @@ More important high-level tests are in contentstore/tests/test_libraries.py
|
||||
import ddt
|
||||
import mock
|
||||
from django.conf import settings
|
||||
from django.urls import reverse
|
||||
from mock import patch
|
||||
from opaque_keys.edx.locator import CourseKey, LibraryLocator
|
||||
from six import binary_type, text_type
|
||||
@@ -342,3 +343,21 @@ class UnitTestLibraries(CourseTestCase):
|
||||
response = self.client.get(manage_users_url)
|
||||
self.assertEqual(response.status_code, 200)
|
||||
self.assertContains(response, extra_user.username)
|
||||
|
||||
def test_component_limits(self):
|
||||
"""
|
||||
Test that component limits in libraries are respected.
|
||||
"""
|
||||
with self.settings(MAX_BLOCKS_PER_CONTENT_LIBRARY=1):
|
||||
library = LibraryFactory.create()
|
||||
data = {
|
||||
'parent_locator': str(library.location),
|
||||
'category': 'html'
|
||||
}
|
||||
response = self.client.ajax_post(reverse('xblock_handler'), data)
|
||||
self.assertEqual(response.status_code, 200)
|
||||
|
||||
# Adding another component should cause failure:
|
||||
response = self.client.ajax_post(reverse('xblock_handler'), data)
|
||||
self.assertEqual(response.status_code, 400)
|
||||
self.assertIn('cannot have more than 1 component', parse_json(response)['error'])
|
||||
|
||||
@@ -2261,3 +2261,6 @@ LOGISTRATION_RATELIMIT_RATE = '100/5m'
|
||||
##### PASSWORD RESET RATE LIMIT SETTINGS #####
|
||||
PASSWORD_RESET_IP_RATE = '1/m'
|
||||
PASSWORD_RESET_EMAIL_RATE = '2/h'
|
||||
|
||||
######################## Setting for content libraries ########################
|
||||
MAX_BLOCKS_PER_CONTENT_LIBRARY = 1000
|
||||
|
||||
@@ -531,6 +531,9 @@ RETIREMENT_SERVICE_WORKER_USERNAME = ENV_TOKENS.get(
|
||||
############### Settings for edx-rbac ###############
|
||||
SYSTEM_WIDE_ROLE_CLASSES = ENV_TOKENS.get('SYSTEM_WIDE_ROLE_CLASSES') or SYSTEM_WIDE_ROLE_CLASSES
|
||||
|
||||
######################## Setting for content libraries ########################
|
||||
MAX_BLOCKS_PER_CONTENT_LIBRARY = ENV_TOKENS.get('MAX_BLOCKS_PER_CONTENT_LIBRARY', MAX_BLOCKS_PER_CONTENT_LIBRARY)
|
||||
|
||||
####################### Plugin Settings ##########################
|
||||
|
||||
# This is at the bottom because it is going to load more settings after base settings are loaded
|
||||
|
||||
@@ -3931,3 +3931,6 @@ GITHUB_REPO_ROOT = '/edx/var/edxapp/data'
|
||||
|
||||
##################### SUPPORT URL ############################
|
||||
SUPPORT_HOW_TO_UNENROLL_LINK = ''
|
||||
|
||||
######################## Setting for content libraries ########################
|
||||
MAX_BLOCKS_PER_CONTENT_LIBRARY = 1000
|
||||
|
||||
@@ -936,6 +936,9 @@ MAINTENANCE_BANNER_TEXT = ENV_TOKENS.get('MAINTENANCE_BANNER_TEXT', None)
|
||||
########################## limiting dashboard courses ######################
|
||||
DASHBOARD_COURSE_LIMIT = ENV_TOKENS.get('DASHBOARD_COURSE_LIMIT', None)
|
||||
|
||||
######################## Setting for content libraries ########################
|
||||
MAX_BLOCKS_PER_CONTENT_LIBRARY = ENV_TOKENS.get('MAX_BLOCKS_PER_CONTENT_LIBRARY', MAX_BLOCKS_PER_CONTENT_LIBRARY)
|
||||
|
||||
############################### Plugin Settings ###############################
|
||||
|
||||
# This is at the bottom because it is going to load more settings after base settings are loaded
|
||||
|
||||
@@ -39,10 +39,12 @@ from uuid import UUID
|
||||
import logging
|
||||
|
||||
import attr
|
||||
from django.conf import settings
|
||||
from django.contrib.auth.models import AbstractUser, Group
|
||||
from django.core.exceptions import PermissionDenied
|
||||
from django.core.validators import validate_unicode_slug
|
||||
from django.db import IntegrityError
|
||||
from django.utils.translation import ugettext as _
|
||||
from lxml import etree
|
||||
from opaque_keys.edx.keys import LearningContextKey
|
||||
from opaque_keys.edx.locator import BundleDefinitionLocator, LibraryLocatorV2, LibraryUsageLocatorV2
|
||||
@@ -95,6 +97,10 @@ class LibraryBlockAlreadyExists(KeyError):
|
||||
""" An XBlock with that ID already exists in the library """
|
||||
|
||||
|
||||
class BlockLimitReachedError(Exception):
|
||||
""" Maximum number of allowed XBlocks in the library reached """
|
||||
|
||||
|
||||
class InvalidNameError(ValueError):
|
||||
""" The specified name/identifier is not valid """
|
||||
|
||||
@@ -513,6 +519,13 @@ def create_library_block(library_key, block_type, definition_id):
|
||||
"""
|
||||
assert isinstance(library_key, LibraryLocatorV2)
|
||||
ref = ContentLibrary.objects.get_by_key(library_key)
|
||||
lib_bundle = LibraryBundle(library_key, ref.bundle_uuid, draft_name=DRAFT_NAME)
|
||||
# Total number of blocks should not exceed the maximum allowed
|
||||
total_blocks = len(lib_bundle.get_top_level_usages())
|
||||
if total_blocks + 1 > settings.MAX_BLOCKS_PER_CONTENT_LIBRARY:
|
||||
raise BlockLimitReachedError(
|
||||
_(u"Library cannot have more than {} XBlocks").format(settings.MAX_BLOCKS_PER_CONTENT_LIBRARY)
|
||||
)
|
||||
# Make sure the proposed ID will be valid:
|
||||
validate_unicode_slug(definition_id)
|
||||
# Ensure the XBlock type is valid and installed:
|
||||
|
||||
@@ -8,6 +8,7 @@ from uuid import UUID
|
||||
from django.contrib.auth.models import Group
|
||||
|
||||
from openedx.core.djangoapps.content_libraries.tests.base import ContentLibrariesRestApiTest
|
||||
from openedx.core.djangoapps.content_libraries.api import BlockLimitReachedError
|
||||
from student.tests.factories import UserFactory
|
||||
|
||||
|
||||
@@ -443,3 +444,18 @@ class ContentLibrariesTest(ContentLibrariesRestApiTest):
|
||||
self.assertEqual(links_created[1]["version"], 1)
|
||||
self.assertEqual(links_created[1]["latest_version"], 2)
|
||||
self.assertEqual(links_created[1]["opaque_key"], bank_lib_id)
|
||||
|
||||
def test_library_blocks_limit(self):
|
||||
"""
|
||||
Test that libraries don't allow more than specified blocks
|
||||
"""
|
||||
with self.settings(MAX_BLOCKS_PER_CONTENT_LIBRARY=1):
|
||||
lib = self._create_library(slug="test_lib_limits", title="Limits Test Library", description="Testing XBlocks limits in a library")
|
||||
lib_id = lib["id"]
|
||||
block_data = self._add_block_to_library(lib_id, "unit", "unit1")
|
||||
# Second block should throw error
|
||||
with self.assertRaises(BlockLimitReachedError):
|
||||
self._add_block_to_library(lib_id, "problem", "problem1")
|
||||
# Also check that limit applies to child blocks too
|
||||
with self.assertRaises(BlockLimitReachedError):
|
||||
self._add_block_to_library(lib_id, "html", "html1", parent_block=block_data["id"])
|
||||
|
||||
Reference in New Issue
Block a user