Skip to content

Commit

Permalink
Refactor rename contentstore xblock services (openedx#32581)
Browse files Browse the repository at this point in the history
This PR addresses the renaming of the contentstore/xblock_services folder to contentstore/xblock_storage_handlers as a follow-up to PR openedx#32282. The renaming is done to prevent naming conflicts with xblock runtime services and to make the purpose of the files more understandable. The file xblock_service.py has been renamed to view_handlers.py to better reflect its functionality.

Justification and Future Refactoring Outlook:
The xblock_storage_handlers folder contains service methods that implement the business logic for view endpoints located in contentstore/views/block.py. It is renamed to xblock_storage_handlers to reflect its responsibility of handling storage-related operations of xblocks, such as creation, retrieval, and deletion.

The view_handlers.py file includes business methods called by the view endpoints. These methods, such as handle_xblock, delete_orphans, etc., interact with the required modulestore methods, handle any errors, and aggregate and serialize data for the response.

The term 'handler' in the context of 'view_handlers.py' represents methods that facilitate business logic for view endpoints. It is critical to note the distinction between these 'handler methods' and the xblock_handler method. The xblock_handler is a view endpoint itself, residing in contentstore/views/block.py, and is well known in this context. Although its name might suggest otherwise, it is not a handler in the sense of the 'handler methods' we've defined in 'view_handlers.py'. To maintain consistency with existing naming conventions, it remains as xblock_handler.
  • Loading branch information
jesperhodge authored Jul 5, 2023
1 parent a68fd49 commit 96f1397
Show file tree
Hide file tree
Showing 46 changed files with 584 additions and 590 deletions.
1 change: 0 additions & 1 deletion cms/djangoapps/contentstore/api/urls.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@


app_name = 'contentstore'
helper = "{0,1}"

urlpatterns = [
re_path(fr'^v0/import/{settings.COURSE_ID_PATTERN}/$',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
from opaque_keys import InvalidKeyError
from opaque_keys.edx.keys import CourseKey

from cms.djangoapps.contentstore.xblock_services.xblock_service import delete_orphans
from cms.djangoapps.contentstore.xblock_storage_handlers.view_handlers import delete_orphans
from xmodule.modulestore import ModuleStoreEnum # lint-amnesty, pylint: disable=wrong-import-order


Expand Down
4 changes: 2 additions & 2 deletions cms/djangoapps/contentstore/rest_api/v1/views/xblock.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,12 @@

from ....api import course_author_access_required

from cms.djangoapps.contentstore.xblock_services import xblock_service
from cms.djangoapps.contentstore.xblock_storage_handlers import view_handlers
import cms.djangoapps.contentstore.toggles as contentstore_toggles

log = logging.getLogger(__name__)
toggles = contentstore_toggles
handle_xblock = xblock_service.handle_xblock
handle_xblock = view_handlers.handle_xblock


@view_auth_classes()
Expand Down
5 changes: 3 additions & 2 deletions cms/djangoapps/contentstore/views/block.py
Original file line number Diff line number Diff line change
Expand Up @@ -43,15 +43,16 @@
)
from .preview import get_preview_fragment

from cms.djangoapps.contentstore.xblock_services import (
from cms.djangoapps.contentstore.xblock_storage_handlers.view_handlers import (
handle_xblock,
create_xblock_info,
load_services_for_studio,
get_block_info,
get_xblock,
delete_orphans,
usage_key_with_run,
)
from cms.djangoapps.contentstore.xblock_storage_handlers.xblock_helpers import usage_key_with_run


__all__ = [
"orphan_handler",
Expand Down
2 changes: 1 addition & 1 deletion cms/djangoapps/contentstore/views/component.py
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@

from ..utils import get_lms_link_for_item, get_sibling_urls, reverse_course_url
from ..helpers import get_parent_xblock, is_unit, xblock_type_display_name
from cms.djangoapps.contentstore.xblock_services.xblock_service import (
from cms.djangoapps.contentstore.xblock_storage_handlers.view_handlers import (
add_container_page_publishing_info,
create_xblock_info,
load_services_for_studio,
Expand Down
2 changes: 1 addition & 1 deletion cms/djangoapps/contentstore/views/course.py
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@
)
from .component import ADVANCED_COMPONENT_TYPES
from ..helpers import is_content_creator
from cms.djangoapps.contentstore.xblock_services.xblock_service import (
from cms.djangoapps.contentstore.xblock_storage_handlers.view_handlers import (
create_xblock_info,
)
from .library import (
Expand Down
4 changes: 2 additions & 2 deletions cms/djangoapps/contentstore/views/entrance_exam.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,8 @@
from xmodule.modulestore.exceptions import ItemNotFoundError # lint-amnesty, pylint: disable=wrong-import-order

from ..helpers import remove_entrance_exam_graders
from ..xblock_services.create_xblock import create_xblock
from cms.djangoapps.contentstore.xblock_services.xblock_service import delete_item
from cms.djangoapps.contentstore.xblock_storage_handlers.create_xblock import create_xblock
from cms.djangoapps.contentstore.xblock_storage_handlers.view_handlers import delete_item

__all__ = ['entrance_exam', ]

Expand Down
2 changes: 1 addition & 1 deletion cms/djangoapps/contentstore/views/library.py
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@
from ..utils import add_instructor, reverse_library_url
from .component import CONTAINER_TEMPLATES, get_component_templates
from ..helpers import is_content_creator
from cms.djangoapps.contentstore.xblock_services.xblock_service import create_xblock_info
from cms.djangoapps.contentstore.xblock_storage_handlers.view_handlers import create_xblock_info
from .user import user_with_role

__all__ = ['library_handler', 'manage_library_users']
Expand Down
6 changes: 3 additions & 3 deletions cms/djangoapps/contentstore/views/tests/test_block.py
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@
duplicate_block,
update_from_source,
)
from cms.djangoapps.contentstore.xblock_services import xblock_service as item_module
from cms.djangoapps.contentstore.xblock_storage_handlers import view_handlers as item_module
from common.djangoapps.student.tests.factories import StaffFactory, UserFactory
from common.djangoapps.xblock_django.models import (
XBlockConfiguration,
Expand All @@ -74,7 +74,7 @@
from openedx.core.djangoapps.discussions.models import DiscussionsConfiguration

from ..component import component_handler, get_component_templates
from cms.djangoapps.contentstore.xblock_services.xblock_service import (
from cms.djangoapps.contentstore.xblock_storage_handlers.view_handlers import (
ALWAYS,
VisibilityState,
get_block_info,
Expand Down Expand Up @@ -1509,7 +1509,7 @@ def test_move_component_nonsensical_access_restriction_validation(self):
validation = html.validate()
self.assertEqual(len(validation.messages), 0)

@patch("cms.djangoapps.contentstore.xblock_services.xblock_service.log")
@patch("cms.djangoapps.contentstore.xblock_storage_handlers.view_handlers.log")
def test_move_logging(self, mock_logger):
"""
Test logging when an item is successfully moved.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@
from xmodule.modulestore.tests.factories import CourseFactory, BlockFactory, LibraryFactory, check_mongo_calls # lint-amnesty, pylint: disable=wrong-import-order

from ..course import _deprecated_blocks_info, course_outline_initial_state, reindex_course_and_check_access
from cms.djangoapps.contentstore.xblock_services.xblock_service import VisibilityState, create_xblock_info
from cms.djangoapps.contentstore.xblock_storage_handlers.view_handlers import VisibilityState, create_xblock_info


class TestCourseIndex(CourseTestCase):
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@
update_entrance_exam
)
from cms.djangoapps.contentstore.helpers import GRADER_TYPES
from cms.djangoapps.contentstore.xblock_services.create_xblock import create_xblock
from cms.djangoapps.contentstore.xblock_storage_handlers.create_xblock import create_xblock


@patch.dict(settings.FEATURES, {'ENTRANCE_EXAMS': True})
Expand Down
16 changes: 8 additions & 8 deletions cms/djangoapps/contentstore/views/tests/test_gating.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
from cms.djangoapps.contentstore.utils import reverse_usage_url
from openedx.core.lib.gating.api import GATING_NAMESPACE_QUALIFIER

from cms.djangoapps.contentstore.xblock_services.xblock_service import VisibilityState
from cms.djangoapps.contentstore.xblock_storage_handlers.view_handlers import VisibilityState


@ddt.ddt
Expand Down Expand Up @@ -57,7 +57,7 @@ def setUp(self):
)
self.seq2_url = reverse_usage_url('xblock_handler', self.seq2.location)

@patch('cms.djangoapps.contentstore.xblock_services.xblock_service.gating_api.add_prerequisite')
@patch('cms.djangoapps.contentstore.xblock_storage_handlers.view_handlers.gating_api.add_prerequisite')
def test_add_prerequisite(self, mock_add_prereq):
"""
Test adding a subsection as a prerequisite
Expand All @@ -69,7 +69,7 @@ def test_add_prerequisite(self, mock_add_prereq):
)
mock_add_prereq.assert_called_with(self.course.id, self.seq1.location)

@patch('cms.djangoapps.contentstore.xblock_services.xblock_service.gating_api.remove_prerequisite')
@patch('cms.djangoapps.contentstore.xblock_storage_handlers.view_handlers.gating_api.remove_prerequisite')
def test_remove_prerequisite(self, mock_remove_prereq):
"""
Test removing a subsection as a prerequisite
Expand All @@ -81,7 +81,7 @@ def test_remove_prerequisite(self, mock_remove_prereq):
)
mock_remove_prereq.assert_called_with(self.seq1.location)

@patch('cms.djangoapps.contentstore.xblock_services.xblock_service.gating_api.set_required_content')
@patch('cms.djangoapps.contentstore.xblock_storage_handlers.view_handlers.gating_api.set_required_content')
def test_add_gate(self, mock_set_required_content):
"""
Test adding a gated subsection
Expand All @@ -100,7 +100,7 @@ def test_add_gate(self, mock_set_required_content):
'100'
)

@patch('cms.djangoapps.contentstore.xblock_services.xblock_service.gating_api.set_required_content')
@patch('cms.djangoapps.contentstore.xblock_storage_handlers.view_handlers.gating_api.set_required_content')
def test_remove_gate(self, mock_set_required_content):
"""
Test removing a gated subsection
Expand All @@ -118,9 +118,9 @@ def test_remove_gate(self, mock_set_required_content):
''
)

@patch('cms.djangoapps.contentstore.xblock_services.xblock_service.gating_api.get_prerequisites')
@patch('cms.djangoapps.contentstore.xblock_services.xblock_service.gating_api.get_required_content')
@patch('cms.djangoapps.contentstore.xblock_services.xblock_service.gating_api.is_prerequisite')
@patch('cms.djangoapps.contentstore.xblock_storage_handlers.view_handlers.gating_api.get_prerequisites')
@patch('cms.djangoapps.contentstore.xblock_storage_handlers.view_handlers.gating_api.get_required_content')
@patch('cms.djangoapps.contentstore.xblock_storage_handlers.view_handlers.gating_api.is_prerequisite')
@ddt.data(
(90, None),
(None, 90),
Expand Down
6 changes: 0 additions & 6 deletions cms/djangoapps/contentstore/xblock_services/__init__.py

This file was deleted.

Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
"""
The xblock_storage_handlers folder contains service methods that implement the business logic for view endpoints
located in contentstore/views/block.py. It is renamed to xblock_storage_handlers to reflect its responsibility
of handling storage-related operations of xblocks, such as creation, retrieval, and deletion.
The view_handlers.py file includes business methods called by the view endpoints.
These methods, such as handle_xblock, delete_orphans, etc., interact with the required modulestore methods,
handle any errors, and aggregate and serialize data for the response.
"""
Original file line number Diff line number Diff line change
Expand Up @@ -116,15 +116,6 @@
NEVER = lambda x: False
ALWAYS = lambda x: True

__all__ = [
"handle_xblock",
"create_xblock_info",
"load_services_for_studio",
"get_block_info",
"get_xblock",
"delete_orphans",
]


def _filter_entrance_exam_grader(graders):
"""
Expand Down
38 changes: 19 additions & 19 deletions conf/locale/ar/LC_MESSAGES/django.po
Original file line number Diff line number Diff line change
Expand Up @@ -373,7 +373,7 @@ msgstr "الرقم التعريفي للفيديو"
msgid "Pending"
msgstr "قيد الانتظار"

#: cms/djangoapps/contentstore/xblock_services/create_xblock.py
#: cms/djangoapps/contentstore/xblock_storage_handlers/create_xblock.py
#: xmodule/library_content_block.py
msgid "Empty"
msgstr "خالي"
Expand Down Expand Up @@ -13065,13 +13065,13 @@ msgid "{previous_groups}, {current_group}"
msgstr "{previous_groups}، {current_group}"

#: cms/djangoapps/contentstore/utils.py
#: cms/djangoapps/contentstore/xblock_services/xblock_service.py
#: cms/djangoapps/contentstore/xblock_storage_handlers/view_handlers.py
#, python-brace-format
msgid "Duplicate of {0}"
msgstr "نسخة مطابقة لـ {0}"

#: cms/djangoapps/contentstore/utils.py
#: cms/djangoapps/contentstore/xblock_services/xblock_service.py
#: cms/djangoapps/contentstore/xblock_storage_handlers/view_handlers.py
#, python-brace-format
msgid "Duplicate of '{0}'"
msgstr "نسخة مطابقة لـ ’{0}‘"
Expand Down Expand Up @@ -13513,66 +13513,66 @@ msgstr "{course}_video_urls"
msgid "A non zero positive integer is expected"
msgstr "عدد موجب غير صفري متوقع"

#: cms/djangoapps/contentstore/xblock_services/xblock_service.py
#: cms/djangoapps/contentstore/xblock_storage_handlers/view_handlers.py
#, python-brace-format
msgid "Libraries cannot have more than {limit} components"
msgstr ""

#: cms/djangoapps/contentstore/xblock_services/xblock_service.py
#: cms/djangoapps/contentstore/xblock_storage_handlers/view_handlers.py
msgid "Invalid data"
msgstr "بيانات غير صالحة"

#: cms/djangoapps/contentstore/xblock_services/xblock_service.py
#: cms/djangoapps/contentstore/xblock_storage_handlers/view_handlers.py
#, python-brace-format
msgid "Invalid data ({details})"
msgstr "بيانات غير صالحة ({details}) "

#: cms/djangoapps/contentstore/xblock_services/xblock_service.py
#: cms/djangoapps/contentstore/xblock_storage_handlers/view_handlers.py
msgid "There was a problem pasting your component."
msgstr ""

#: cms/djangoapps/contentstore/xblock_services/xblock_service.py
#: cms/djangoapps/contentstore/xblock_storage_handlers/view_handlers.py
msgid "Your clipboard is empty or invalid."
msgstr ""

#: cms/djangoapps/contentstore/xblock_services/xblock_service.py
#: cms/djangoapps/contentstore/xblock_storage_handlers/view_handlers.py
#, python-brace-format
msgid "You can not move {source_type} into {target_parent_type}."
msgstr "لا يمكنك نقل {source_type} إلى {target_parent_type}."

#: cms/djangoapps/contentstore/xblock_services/xblock_service.py
#: cms/djangoapps/contentstore/xblock_storage_handlers/view_handlers.py
msgid "Item is already present in target location."
msgstr "العنصر موجود من قبل في الموقع الهدف."

#: cms/djangoapps/contentstore/xblock_services/xblock_service.py
#: cms/djangoapps/contentstore/xblock_storage_handlers/view_handlers.py
msgid "You can not move an item into itself."
msgstr "لا يمكنك نقل عنصر إلى نفسه."

#: cms/djangoapps/contentstore/xblock_services/xblock_service.py
#: cms/djangoapps/contentstore/xblock_storage_handlers/view_handlers.py
msgid "You can not move an item into it's child."
msgstr "لا يمكنك نقل عنصر إلى فرعه."

#: cms/djangoapps/contentstore/xblock_services/xblock_service.py
#: cms/djangoapps/contentstore/xblock_storage_handlers/view_handlers.py
msgid "You can not move an item directly into content experiment."
msgstr "لا يمكنك نقل عنصر مباشرة إلى تجربة المحتوى."

#: cms/djangoapps/contentstore/xblock_services/xblock_service.py
#: cms/djangoapps/contentstore/xblock_storage_handlers/view_handlers.py
#, python-brace-format
msgid "{source_usage_key} not found in {parent_usage_key}."
msgstr "{source_usage_key} غير موجود في {parent_usage_key}."

#: cms/djangoapps/contentstore/xblock_services/xblock_service.py
#: cms/djangoapps/contentstore/xblock_storage_handlers/view_handlers.py
#, python-brace-format
msgid ""
"You can not move {source_usage_key} at an invalid index ({target_index})."
msgstr "لا يمكنك نقل {source_usage_key} في فهرس غير صالح ({target_index})."

#: cms/djangoapps/contentstore/xblock_services/xblock_service.py
#: cms/djangoapps/contentstore/xblock_storage_handlers/view_handlers.py
#, python-brace-format
msgid "You must provide target_index ({target_index}) as an integer."
msgstr "يجب إدخال target_index ({target_index}) كعدد صحيح."

#: cms/djangoapps/contentstore/xblock_services/xblock_service.py
#: cms/djangoapps/contentstore/xblock_storage_handlers/view_handlers.py
#, python-brace-format
msgid ""
"Students must score {score}{pct_sign} or higher to access course materials."
Expand All @@ -13582,11 +13582,11 @@ msgstr ""

#. Translators: This is the percent sign. It will be used to represent
#. a percent value out of 100, e.g. "58%" means "58/100".
#: cms/djangoapps/contentstore/xblock_services/xblock_service.py
#: cms/djangoapps/contentstore/xblock_storage_handlers/view_handlers.py
msgid "%"
msgstr "%"

#: cms/djangoapps/contentstore/xblock_services/xblock_service.py
#: cms/djangoapps/contentstore/xblock_storage_handlers/view_handlers.py
#, python-brace-format
msgid "{section_or_subsection} \"{display_name}\""
msgstr "{section_or_subsection} \"{display_name}\""
Expand Down
Loading

0 comments on commit 96f1397

Please sign in to comment.