Merge pull request #204 from openedx/farhan/remove-legacy-attribs

Remove XModuleMixin legacy attibs from wordcloud, video block, lti block

Author: farhan

xblocks_contrib/common/xml_utils.py

@@ -139,7 +139,7 @@ def own_metadata(block: XBlock) -> dict[str, Any]:
             except TypeError as exception:
                 exception_message = "{message}, Block-location:{location}, Field-name:{field_name}".format(
                     message=str(exception),
-                    location=str(block.location),
+                    location=str(block.usage_key),
                     field_name=field.name
                 )
                 raise TypeError(exception_message)  # lint-amnesty, pylint: disable=raise-missing-from
@@ -486,12 +486,12 @@ def add_xml_to_node(self, node):
                 aside.add_xml_to_node(aside_node)
                 xml_object.append(aside_node)
 
-        not_to_clean_fields = self.metadata_to_not_to_clean.get(self.category, ())
+        not_to_clean_fields = self.metadata_to_not_to_clean.get(self.usage_key.block_type, ())
         self.clean_metadata_from_xml(xml_object, excluded_fields=not_to_clean_fields)
 
         # Set the tag on both nodes so we get the file path right.
-        xml_object.tag = self.category
-        node.tag = self.category
+        xml_object.tag = self.usage_key.block_type
+        node.tag = self.usage_key.block_type
 
         # Add the non-inherited metadata
         for attr in sorted(own_metadata(self)):
@@ -506,7 +506,7 @@ def add_xml_to_node(self, node):
                     logging.exception(
                         'Failed to serialize metadata attribute %s with value %s in module %s. '
                         'This could mean data loss!!!',
-                        attr, val, self.url_name
+                        attr, val, self.usage_key.block_id
                     )
 
         for key, value in self.xml_attributes.items():
@@ -515,8 +515,8 @@ def add_xml_to_node(self, node):
 
         if self.export_to_file():
             # Write the definition to a file
-            url_path = name_to_pathname(self.url_name)
-            filepath = self._format_filepath(self.category, url_path)
+            url_path = name_to_pathname(self.usage_key.block_id)
+            filepath = self._format_filepath(self.usage_key.block_type, url_path)
             self.runtime.export_fs.makedirs(os.path.dirname(filepath), recreate=True)
             with self.runtime.export_fs.open(filepath, 'wb') as fileobj:
                 ElementTree(xml_object).write(fileobj, pretty_print=True, encoding='utf-8')
@@ -531,16 +531,16 @@ def add_xml_to_node(self, node):
 
         # Do not override an existing value for the course.
         if not node.get('url_name'):
-            node.set('url_name', self.url_name)
+            node.set('url_name', self.usage_key.block_id)
 
         # We do not need to cater the `course` category here in xblocks_contrib,
         # because course export is handled in the edx-platform code.
 
         # Special case for course pointers:
-        # if self.category == 'course':
+        # if self.usage_key.block_type == 'course':
         #     # add org and course attributes on the pointer tag
-        #     node.set('org', self.location.org)
-        #     node.set('course', self.location.course)
+        #     node.set('org', self.usage_key.org)
+        #     node.set('course', self.usage_key.course)
 
     def definition_to_xml(self, resource_fs):
         """

xblocks_contrib/discussion/discussion.py

@@ -150,7 +150,7 @@ def add_resource_urls(self, fragment):
         fragment.add_css_url(self.runtime.local_resource_url(self, f"{base_path}{css_file_path}"))
         fragment.add_javascript_url(self.runtime.local_resource_url(self, f"{base_path}/js/discussion_bundle.js"))
 
-    def has_permission(self, permission):  # pylint: disable=unused-argument
+    def has_permission(self, permission):
         """
         Encapsulates lms specific functionality, as `has_permission` is not
         importable outside of lms context, namely in tests.

xblocks_contrib/lti/lti.py

@@ -70,7 +70,7 @@
 from django.utils.translation import gettext_noop as _
 from lxml import etree
 from oauthlib.oauth1.rfc5849 import signature
-from opaque_keys.edx.keys import CourseKey, UsageKey
+from opaque_keys.edx.keys import CourseKey
 from pytz import UTC
 from web_fragments.fragment import Fragment
 from webob import Response
@@ -378,27 +378,6 @@ class LTIBlock(
     # Indicates that this XBlock has been extracted from edx-platform.
     is_extracted = True
 
-    @property
-    def category(self):
-        """Return the block type/category."""
-        return self.scope_ids.block_type
-
-    @property
-    def url_name(self):
-        return self.location.block_id
-
-    @property
-    def location(self):
-        return self.scope_ids.usage_id
-
-    @location.setter
-    def location(self, value):
-        assert isinstance(value, UsageKey)
-        self.scope_ids = self.scope_ids._replace(
-            def_id=value,  # Note: assigning a UsageKey as def_id is OK in old mongo / import system but wrong in split
-            usage_id=value,
-        )
-
     def max_score(self):
         return self.weight if self.has_score else None
 
@@ -513,7 +492,7 @@ def get_context(self):
 
             # These parameters do not participate in OAuth signing.
             'launch_url': self.launch_url.strip(),
-            'element_id': self.location.html_id(),
+            'element_id': self.usage_key.html_id(),
             'element_class': self.scope_ids.block_type,
             'open_in_a_new_page': self.open_in_a_new_page,
             'display_name': self.display_name,
@@ -747,7 +726,7 @@ def get_resource_link_id(self):
         i4x-2-3-lti-31de800015cf4afb973356dbe81496df this part of resource_link_id:
         makes resource_link_id to be unique among courses inside same system.
         """
-        return str(parse.quote(f"{settings.LMS_BASE}-{self.location.html_id()}"))
+        return str(parse.quote(f"{settings.LMS_BASE}-{self.usage_key.html_id()}"))
 
     def get_lis_result_sourcedid(self):
         """
@@ -773,8 +752,8 @@ def get_course(self):
               In general, please do not add new code that access Modulestore, because it
               will not work with openedx_content. We do it here just to support a legacy feature.
         """
-        if isinstance(self.location.course_key, CourseKey):
-            return self.runtime.modulestore.get_course(self.location.course_key)
+        if isinstance(self.context_key, CourseKey):
+            return self.runtime.modulestore.get_course(self.context_key)
         return None
 
     @property
@@ -785,7 +764,7 @@ def context_id(self):
         context_id is an opaque identifier that uniquely identifies the context (e.g., a course)
         that contains the link being launched.
         """
-        return str(self.location.course_key)
+        return str(self.context_key)
 
     @property
     def role(self):
@@ -882,8 +861,8 @@ def oauth_params(self, custom_parameters, client_key, client_secret):
             # Stubbing headers for now:
             log.info(
                 "LTI block %s in course %s does not have oauth parameters correctly configured.",
-                self.location,
-                self.location.course_key,
+                self.usage_key,
+                self.context_key,
             )
             headers = {
                 'Content-Type': 'application/x-www-form-urlencoded',
@@ -1037,7 +1016,7 @@ def definition_from_xml(cls, xml_object, system):
     def definition_to_xml(self, resource_fs):
         if self.data:
             return etree.fromstring(self.data)
-        return etree.Element(self.category)
+        return etree.Element(self.usage_key.block_type)
 
     def bind_for_student(self, user_id, wrappers=None):
         """

xblocks_contrib/lti/tests/test_lti_unit.py

@@ -59,7 +59,7 @@ def setUp(self):
                   </imsx_POXBody>
                 </imsx_POXEnvelopeRequest>
             """)
-        self.course_id = CourseKey.from_string('org/course/run')
+        self.context_key = CourseKey.from_string('org/course/run')
         self.runtime = get_test_system()
         self.runtime.publish = Mock()
         self.runtime._services['rebind_user'] = Mock()  # pylint: disable=protected-access
@@ -68,7 +68,7 @@ def setUp(self):
             self.runtime,
             DictFieldData({}),
             ScopeIds(
-                None, None, None, BlockUsageLocator(self.course_id, "lti", "name")
+                None, None, None, BlockUsageLocator(self.context_key, "lti", "name")
             ),
         )
         current_user = self.runtime.service(self.xblock, "user").get_current_user()
@@ -326,9 +326,9 @@ def mock_handler_url(
 
     def test_resource_link_id(self):
         with patch(
-            "xblocks_contrib.lti.lti.LTIBlock.location", new_callable=PropertyMock
+            "xblocks_contrib.lti.lti.LTIBlock.usage_key", new_callable=PropertyMock
         ):
-            self.xblock.location.html_id = (
+            self.xblock.usage_key.html_id = (
                 lambda: "i4x-2-3-lti-31de800015cf4afb973356dbe81496df"
             )
             expected_resource_link_id = str(parse.quote(self.unquoted_resource_link_id))
@@ -339,7 +339,7 @@ def test_lis_result_sourcedid(self):
         expected_sourced_id = ":".join(
             parse.quote(i)
             for i in (
-                str(self.course_id),
+                str(self.context_key),
                 self.xblock.get_resource_link_id(),
                 self.user_id,
             )
@@ -576,4 +576,4 @@ def test_context_id(self):
         """
         Tests that LTI parameter context_id is equal to course_id.
         """
-        assert str(self.course_id) == self.xblock.context_id
+        assert str(self.context_key) == self.xblock.context_id

xblocks_contrib/video/content.py

@@ -765,7 +765,7 @@ def test_export_to_xml(self, mock_get_edxval_api):
             video_id=edx_video_id,
             static_dir=EXPORT_IMPORT_STATIC_DIR,
             resource_fs=self.file_system,
-            course_id=self.block.scope_ids.usage_id.context_key,
+            course_id=self.block.context_key,
         )
 
     def test_export_to_xml_without_video_id(self):