Skip to content

Commit ce00b16

Browse files
test: Enforce application layering using import linter (#36581)
* test: add import linters to promote clean dependencies * docs: update a few comments / todos
1 parent 9d45f85 commit ce00b16

4 files changed

Lines changed: 41 additions & 1 deletion

File tree

mypy.ini

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,8 @@ files =
99
cms/lib/xblock/upstream_sync.py,
1010
cms/djangoapps/contentstore/rest_api/v2/views/downstreams.py,
1111
openedx/core/djangoapps/content/learning_sequences,
12+
# FIXME: need to solve type issues and add 'search' app here:
13+
# openedx/core/djangoapps/content/search,
1214
openedx/core/djangoapps/content_staging,
1315
openedx/core/djangoapps/content_libraries,
1416
openedx/core/djangoapps/programs/rest_api,

openedx/core/djangoapps/content_staging/models.py

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -131,7 +131,6 @@ def get_source_context_title(self) -> str:
131131

132132
def clean(self):
133133
""" Check that this model is being used correctly. """
134-
# These could probably be replaced with constraints in Django 4.1+
135134
if self.user.id != self.content.user.id:
136135
raise ValidationError("User ID mismatch.")
137136
if self.content.purpose != CLIPBOARD_PURPOSE:

openedx/core/djangoapps/content_staging/views.py

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,12 @@ def get(self, request):
8181
def post(self, request):
8282
"""
8383
Put some piece of content into the user's clipboard.
84+
85+
FIXME: This API needs to be deprecated and replaced by dedicated APIs
86+
within each learning context (POST /course/foo/bar/copy, POST
87+
/library/foo/bar/copy, etc.) We don't want to encode course- and
88+
library-specific logic in content staging, and it shouldn't import
89+
course or library modules.
8490
"""
8591
# Check if the content exists and the user has permission to read it.
8692
# Parse the usage key:

setup.cfg

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -194,3 +194,36 @@ source_modules =
194194
forbidden_modules =
195195
openedx_learning.apps
196196
allow_indirect_imports = True
197+
198+
[importlinter:contract:4]
199+
name = Low-level apps should not depend on high-level apps
200+
type = layers
201+
layers =
202+
# Layers from high-level to low-level. Imports should only occur from higher to lower.
203+
cms.lib.xblock.upstream_sync | openedx.core.djangoapps.content.search | openedx.core.djangoapps.olx_rest_api
204+
openedx.core.djangoapps.content_libraries
205+
openedx.core.djangoapps.content_staging
206+
openedx.core.djangoapps.xblock
207+
openedx.core.lib.xblock_serializer
208+
openedx.core.djangoapps.content_tagging
209+
ignore_imports =
210+
# Test code can break these layering rules
211+
**.tests.** -> **
212+
213+
# FIXME: the exceptions below are from before we added this import linting rule. Should refactor to eliminate them.
214+
# In particular, the contentstore.helpers module is too big and has too many imports - split it up?
215+
216+
# The CSV export hard-codes support for courses and libraries. Refactor to do something like learning_context.get_children()
217+
openedx.core.djangoapps.content_tagging.helpers.objecttag_export_helpers -> openedx.core.djangoapps.content_libraries.api
218+
# The permissions checking code for tagging requires libraries model data to get all the orgs that a user is using:
219+
openedx.core.djangoapps.content_tagging.utils -> openedx.core.djangoapps.content_libraries.api
220+
# Content staging POST to clipboard API uses libraries APIs. We're working on moving this code to content_libraries
221+
openedx.core.djangoapps.content_staging.views -> openedx.core.djangoapps.content_libraries.api
222+
# content_staging.serializers imports contentstore.helpers which imports contentstore.utils which imports the libraries API.
223+
openedx.core.djangoapps.content_staging.serializers -> cms.djangoapps.contentstore.helpers
224+
# content_libraries.rest_api.libraries imports cms.djangoapps.contentstore.views.course which imports
225+
# contentstore.toggles which imports djangoapps.content.search.api
226+
openedx.core.djangoapps.content_libraries.rest_api.libraries -> cms.djangoapps.contentstore.views.course
227+
# Content libraries imports contentstore.helpers which imports upstream_sync
228+
openedx.core.djangoapps.content_libraries.api.blocks -> cms.djangoapps.contentstore.helpers
229+
openedx.core.djangoapps.content_libraries.api.libraries -> cms.djangoapps.contentstore.helpers

0 commit comments

Comments
 (0)