Skip to content

Commit a7f45fc

Browse files
authored
refactor: unify authoring apps into openedx_content (#454)
This commit is a major refactoring that does the following: - All existing authoring apps (backup_restore, collections, components, contents, publishing, sections, subsections, units) are unified into the new openedx_content app. - The logical separation between the previous apps are preserved as packages in a new "applets" subpackage inside of openedx_content. Each applets subpackage has its own models.py and api.py modules. - Skeletons of the old apps are preserved in the new "backcompat" package inside of openedx_content. This is done to preserve database migration compatibility. - Increments the version to 0.31.0 The motivation for this change is to make it easier to refactor apps going forward. Having entirely separate apps makes it difficult to make any changes that involve moving models across apps. This has come up a couple of times already, when trying to extract container logic out of publishing and when trying to rename the "contents" app to "media". The name change away from "authoring" also reflects that we intend to use these APIs and models on the "learning" side of things as well, e.g. when reading content for rendering to a student. In the longer term, we will probably make openedx_content a top-level package in this repo, but that would be a later step. Most of this commit is just shuffling things around and renaming references, but the truly complicated bits are around the sequencing of database migrations, particularly the ones removing model state from the old apps, and transferring that state into the new openedx_content app without changing the actual database state. All this is explained in greater detail in the following ADR: docs/decisions/0020-merge-authoring-apps-into-openedx-content.rst
1 parent 33985a5 commit a7f45fc

File tree

182 files changed

+2278
-433
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

182 files changed

+2278
-433
lines changed

.annotation_safe_list.yml

Lines changed: 25 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -19,43 +19,43 @@ auth.User:
1919
".. pii_retirement": "consumer_api"
2020
contenttypes.ContentType:
2121
".. no_pii:": "This model has no PII"
22-
oel_collections.Collection:
22+
openedx_content.Collection:
2323
".. no_pii:": "This model has no PII"
24-
oel_collections.CollectionPublishableEntity:
24+
openedx_content.CollectionPublishableEntity:
2525
".. no_pii:": "This model has no PII"
26-
oel_components.Component:
26+
openedx_content.Component:
2727
".. no_pii:": "This model has no PII"
28-
oel_components.ComponentType:
28+
openedx_content.ComponentType:
2929
".. no_pii:": "This model has no PII"
30-
oel_components.ComponentVersion:
30+
openedx_content.ComponentVersion:
3131
".. no_pii:": "This model has no PII"
32-
oel_components.ComponentVersionContent:
32+
openedx_content.ComponentVersionContent:
3333
".. no_pii:": "This model has no PII"
34-
oel_contents.Content:
34+
openedx_content.Content:
3535
".. no_pii:": "This model has no PII"
36-
oel_contents.MediaType:
36+
openedx_content.MediaType:
3737
".. no_pii:": "This model has no PII"
38-
oel_publishing.Container:
38+
openedx_content.Container:
3939
".. no_pii:": "This model has no PII"
40-
oel_publishing.ContainerVersion:
40+
openedx_content.ContainerVersion:
4141
".. no_pii:": "This model has no PII"
42-
oel_publishing.Draft:
42+
openedx_content.Draft:
4343
".. no_pii:": "This model has no PII"
44-
oel_publishing.EntityList:
44+
openedx_content.EntityList:
4545
".. no_pii:": "This model has no PII"
46-
oel_publishing.EntityListRow:
46+
openedx_content.EntityListRow:
4747
".. no_pii:": "This model has no PII"
48-
oel_publishing.LearningPackage:
48+
openedx_content.LearningPackage:
4949
".. no_pii:": "This model has no PII"
50-
oel_publishing.PublishLog:
50+
openedx_content.PublishLog:
5151
".. no_pii:": "This model has no PII"
52-
oel_publishing.PublishLogRecord:
52+
openedx_content.PublishLogRecord:
5353
".. no_pii:": "This model has no PII"
54-
oel_publishing.PublishableEntity:
54+
openedx_content.PublishableEntity:
5555
".. no_pii:": "This model has no PII"
56-
oel_publishing.PublishableEntityVersion:
56+
openedx_content.PublishableEntityVersion:
5757
".. no_pii:": "This model has no PII"
58-
oel_publishing.Published:
58+
openedx_content.Published:
5959
".. no_pii:": "This model has no PII"
6060
oel_tagging.ObjectTag:
6161
".. no_pii:": "This model has no PII"
@@ -65,17 +65,17 @@ oel_tagging.TagImportTask:
6565
".. no_pii:": "This model has no PII"
6666
oel_tagging.Taxonomy:
6767
".. no_pii:": "This model has no PII"
68-
oel_sections.Section:
68+
openedx_content.Section:
6969
".. no_pii:": "This model has no PII"
70-
oel_sections.SectionVersion:
70+
openedx_content.SectionVersion:
7171
".. no_pii:": "This model has no PII"
72-
oel_subsections.Subsection:
72+
openedx_content.Subsection:
7373
".. no_pii:": "This model has no PII"
74-
oel_subsections.SubsectionVersion:
74+
openedx_content.SubsectionVersion:
7575
".. no_pii:": "This model has no PII"
76-
oel_units.Unit:
76+
openedx_content.Unit:
7777
".. no_pii:": "This model has no PII"
78-
oel_units.UnitVersion:
78+
openedx_content.UnitVersion:
7979
".. no_pii:": "This model has no PII"
8080
social_django.Association:
8181
".. no_pii:": "This model has no PII"

.gitignore

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,12 @@ venv/
7575
!.vscode/settings.json.example
7676

7777
# Media files (for uploads)
78-
media/
78+
/media/
7979

8080
# Media files generated during test runs
81-
test_media/
81+
/test_media/
82+
83+
# uv stuff
84+
.lock
85+
CACHEDIR.TAG
86+
pyvenv.cfg

.importlinter

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -36,24 +36,24 @@ layers=
3636
openedx_learning.api.authoring
3737

3838
# The "backup_restore" app handle the new export and import mechanism.
39-
openedx_learning.apps.authoring.backup_restore
39+
openedx_learning.apps.openedx_content.applets.backup_restore
4040

4141
# The "components" app is responsible for storing versioned Components,
4242
# which is Open edX Studio terminology maps to things like individual
4343
# Problems, Videos, and blocks of HTML text. This is also the type we would
4444
# associate with a single "leaf" XBlock–one that is not a container type and
4545
# has no child elements.
46-
openedx_learning.apps.authoring.components
46+
openedx_learning.apps.openedx_content.applets.components
4747

4848
# The "contents" app stores the simplest pieces of binary and text data,
4949
# without versioning information. These belong to a single Learning Package.
50-
openedx_learning.apps.authoring.contents
50+
openedx_learning.apps.openedx_content.applets.contents
5151

5252
# The "collections" app stores arbitrary groupings of PublishableEntities.
5353
# Its only dependency should be the publishing app.
54-
openedx_learning.apps.authoring.collections
54+
openedx_learning.apps.openedx_content.applets.collections
5555

5656
# The lowest layer is "publishing", which holds the basic primitives needed
5757
# to create Learning Packages and manage the draft and publish states for
5858
# various types of content.
59-
openedx_learning.apps.authoring.publishing
59+
openedx_learning.apps.openedx_content.applets.publishing
Lines changed: 87 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,87 @@
1+
20. Merge authoring apps into openedx_content (using Applets)
2+
=============================================================
3+
4+
Context
5+
-------
6+
7+
Up to this point, Learning Core has used many small apps with a narrow focus (e.g. ``components``, ``collections``, etc.) in order to make each individual app simpler to reason about. This has been useful overall, but it has made refactoring more cumbersome. For instance:
8+
9+
#. Moving models between apps is tricky, requiring the use of Django's ``SeparateDatabaseAndState`` functionality to fake a deletion in one app and a creation in another without actually altering the database. Moving models also introduces tricky dependencies with respect to migration ordering (described in more detail later in this document). We encountered this when considering how to extract container functionality out of the ``publishing`` app.
10+
#. Renaming an app is also cumbersome, because the process requires creating a new app and transitioning the models over. This came up when trying to rename the ``contents`` app to ``media``.
11+
12+
There have also been minor inconveniences, like having a long list of ``INSTALLED_APPS`` to maintain in openedx-platform over time, or not having these tables easily grouped together in the Django admin interface.
13+
14+
Decisions
15+
---------
16+
17+
1. Single openedx_content App
18+
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
19+
20+
All existing authoring apps will be merged into one Django app named ``openedx_content``. Some consequences of this decision:
21+
22+
- The tables will be renamed to have the ``openedx_content`` label prefix.
23+
- All management commands will be moved to the ``openedx_content`` app.
24+
25+
2. Logical Separation via Applets
26+
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
27+
28+
We will continue to keep internal API boundaries by using a new "Applets" convention. A Django applet is made to look like a miniature Django app, with its own ``models.py``, ``api.py``, and potentially other modules. The modules for the old authoring apps will be copied into various subpackages of ``openedx_content.applets``, such as ``openedx_content.applets.publishing``. Applets should respect each others' API boundaries, and never directly query models across applets. As before, we will use Import Linter to enforce dependency ordering.
29+
30+
3. Package Restructuring
31+
~~~~~~~~~~~~~~~~~~~~~~~~
32+
33+
In one pull request, we are going to:
34+
35+
#. Rename the ``openedx_learning.apps.authoring`` package to be ``openedx_learning.apps.openedx_content``. (Note: We have discussed eventually moving this to a top level ``openedx_content`` app instead of ``openedx_learning.apps.openedx_content``, but that will happen at a later time.)
36+
#. Create bare shells of the existing ``authoring`` apps (``backup_restore``, ``collections``, ``components``, ``contents``, ``publishing``, ``sections``, ``subsections``, ``units``), and move them to the ``openedx_learning.apps.openedx_content.backcompat`` package. These shells will have an ``apps.py`` file, the ``migrations`` package for each existing app, and in some cases a minimal ``models.py`` that will hold the skeletons of a handful of models. This will allow for a smooth schema migration to transition the models from these individual apps to ``openedx_content``.
37+
#. Move the actual models files and API logic for our existing authoring apps to the ``openedx_learning.apps.openedx_content.applets`` package.
38+
#. Convert the top level ``openedx_learning.apps.openedx_content`` package to be a Django app. The top level ``admin.py``, ``api.py``, and ``models.py`` modules will do wildcard imports from the corresponding modules across all applet packages.
39+
#. Test packages will also be updated to follow the new structure.
40+
41+
4. Database Migration Specifics
42+
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
43+
44+
When Django runs migrations, it both:
45+
46+
* Calculates an ephemeral logical model **state**, based on the contents of the Python migration files and the ``django_migration`` database table, which indicates the list of migrations that have been "run".
47+
* Actually executes the migration operations on the app **database** tables as each migration is "run".
48+
49+
We are going to take advantage of the fact that these two can be separated using the ``SeparateDatabaseAndState`` operation. We will use this to remove the model state from the old authoring apps and create the model state in the new ``openedx_content`` app without having to run database operations.
50+
51+
There are a few high level constraints that we have to consider:
52+
53+
#. Existing openedx-platform migrations should not be modified. Existing openedx-platform migrations should remain unchanged. This is important to make sure that we do not introduce ordering inconsistencies for sites that have already run migrations for the old apps and are upgrading to a new release (e.g. Verawood).
54+
#. The openedx-learning repo should not have any dependencies on openedx-platform migrations, because our dependencies strictly go in the other direction: openedx-platform calls openedx-learning, not the other way around. Furthermore, openedx-learning will often be run without openedx-platform, such as for local development or during CI.
55+
#. Two of the openedx-platform apps that have foreign keys to openedx-learning models are only in Studio's INSTALLED_APPS (``contentstore`` and ``modulestore_migrator``), while ``content_libraries`` is installed in both Studio and LMS. Migrations may be run for LMS or Studio first, depending on the user and environment. Tutor runs LMS first, but we can't assume that will always be true.
56+
#. We must support people who are installing from scratch, those who are upgrading from the Ulmo release, as well as those who are running off of the master branch of openedx-platform.
57+
58+
Therefore, the migrations will happen in the following order:
59+
60+
#. All pre-existing ``backcompat.*`` apps migrations run as before.
61+
#. New ``backcompat.*`` apps migrations that drop most model state, but leave the database unchanged.
62+
#. The first ``openedx_content`` migration creates logical models without any database changes.
63+
#. The second ``openedx_content`` migration renames the underlying tables.
64+
#. Each the ``openedx-platform`` apps that had foreign keys to the old authoring apps will get a new migration that switches those foreign keys to point to ``openedx_content`` apps instead. These are: ``content_libraries``, ``contentstore``, and ``modulestore_migrator``.
65+
#. The above ``openedx-platform`` apps will also get a squashed migration that sets them up to point to the new ``openedx_content`` models directly.
66+
67+
The tricky part is that all the ``opendx-learning`` migrations will run before any of the ``openedx-platform`` migrations run. We can't force it to do otherwise without making ``openedx-learning`` aware of ``opendx-platform``, and we explicitly want to avoid that. This makes things tricky with respect to the model state dependencies. There are two scenarios we have to worry about:
68+
69+
Migration from Scrach
70+
The ``openedx-platform`` apps will each run the squashed migration that jumps straight to making foreign keys against the new ``openedx_content`` models, so the fact that the old authoring app models have been removed and the tables have been renamed doesn't matter.
71+
72+
Migration from Ulmo/master
73+
No actual database operations have to happen here, as the keys were already created earlier. That being said, the migration framework will error out if the state of the old app models that it had foreign keys to have been dropped entirely. That's why the bare skeletons of those old models are preserved in the ``backcompat`` app models files, along with their primary key field. Everything else can be dropped from the state point of view—though again, we're not modifying database state in this operation.
74+
75+
The main downside of this approach is that it may break migrations for developers if they have a months old dev database that is in an in-between release state, e.g. after some ``modulestore_migrations`` referencing the old app mdoels were run, but before the most recent ``modulestore_migrations`` creating foreign keys to those models. No production environment is expected to deploy like this, so this would mainly impact developers. The workaround to this would be to run ``python manage.py lms migrate openedx_content 0001`` to rename the tables to their old values, then run the failing ``contentstore``, ``content_libraries``, or ``modulestore_migrator`` migrations again.
76+
77+
5. Rejected Alternatives
78+
~~~~~~~~~~~~~~~~~~~~~~~~
79+
80+
An earlier attempt at this tried to solve the migration ordering issue by dynamically injecting migration dependencies into the second ``openedx_content`` migration module during the app config ``ready()`` initialization. This was later abandoned because it didn't solve the problem of CMS vs LMS differences in ``INSTALLED_APPS``, so the ordering could still get corrupted unless we added those apps to LMS—which would have introduced more risk.
81+
82+
It's also worth noting that Django startup checks will fail if it detects that multiple models point to the same table. This is why we rename the tables for the ``openedx_contents`` models, and leave the skeleton models in ``backcompat`` apps pointing to the old table names (which no longer really exist in the database once these migrations run).
83+
84+
6. The Bigger Picture
85+
~~~~~~~~~~~~~~~~~~~~~
86+
87+
This overall refactoring means that the ``openedx_content`` Django app corresponds to a Subdomain in Domain Driven Design terminology, with each applet being a Bounded Context. We call these "Applets" instead of "Bounded Contexts" because we don't want it to get confused for Django's notion of Contexts and Context Processors (or Python's notion of Context Managers).

olx_importer/management/commands/load_components.py

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -28,9 +28,9 @@
2828
from django.db import transaction
2929

3030
# Model references to remove
31-
from openedx_learning.apps.authoring.components import api as components_api
32-
from openedx_learning.apps.authoring.contents import api as contents_api
33-
from openedx_learning.apps.authoring.publishing import api as publishing_api
31+
from openedx_learning.apps.openedx_content.applets.components import api as components_api
32+
from openedx_learning.apps.openedx_content.applets.contents import api as contents_api
33+
from openedx_learning.apps.openedx_content.applets.publishing import api as publishing_api
3434

3535
SUPPORTED_TYPES = ["problem", "video", "html"]
3636
logger = logging.getLogger(__name__)

openedx_learning/__init__.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,4 +2,4 @@
22
Open edX Learning ("Learning Core").
33
"""
44

5-
__version__ = "0.30.2"
5+
__version__ = "0.31.0"

openedx_learning/api/authoring.py

Lines changed: 2 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -2,21 +2,14 @@
22
This is the public API for content authoring in Learning Core.
33
44
This is the single ``api`` module that code outside of the
5-
``openedx_learning.apps.authoring.*`` package should import from. It will
5+
``openedx_learning.apps.openedx_content.*`` package should import from. It will
66
re-export the public functions from all api.py modules of all authoring apps. It
77
may also implement its own convenience APIs that wrap calls to multiple app
88
APIs.
99
"""
1010
# These wildcard imports are okay because these api modules declare __all__.
1111
# pylint: disable=wildcard-import
12-
from ..apps.authoring.backup_restore.api import *
13-
from ..apps.authoring.collections.api import *
14-
from ..apps.authoring.components.api import *
15-
from ..apps.authoring.contents.api import *
16-
from ..apps.authoring.publishing.api import *
17-
from ..apps.authoring.sections.api import *
18-
from ..apps.authoring.subsections.api import *
19-
from ..apps.authoring.units.api import *
12+
from ..apps.openedx_content.api import *
2013

2114
# This was renamed after the authoring API refactoring pushed this and other
2215
# app APIs into the openedx_learning.api.authoring module. Here I'm aliasing to

openedx_learning/api/authoring_models.py

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -7,10 +7,10 @@
77
"""
88
# These wildcard imports are okay because these modules declare __all__.
99
# pylint: disable=wildcard-import
10-
from ..apps.authoring.collections.models import *
11-
from ..apps.authoring.components.models import *
12-
from ..apps.authoring.contents.models import *
13-
from ..apps.authoring.publishing.models import *
14-
from ..apps.authoring.sections.models import *
15-
from ..apps.authoring.subsections.models import *
16-
from ..apps.authoring.units.models import *
10+
from ..apps.openedx_content.applets.collections.models import *
11+
from ..apps.openedx_content.applets.components.models import *
12+
from ..apps.openedx_content.applets.contents.models import *
13+
from ..apps.openedx_content.applets.publishing.models import *
14+
from ..apps.openedx_content.applets.sections.models import *
15+
from ..apps.openedx_content.applets.subsections.models import *
16+
from ..apps.openedx_content.applets.units.models import *

openedx_learning/api/django.py

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
"""
2+
Module for parts of the Learning Core API that exist to make it easier to use in
3+
Django projects.
4+
"""
5+
6+
7+
def openedx_learning_apps_to_install():
8+
"""
9+
Return all app names for appending to INSTALLED_APPS.
10+
11+
This function exists to better insulate edx-platform and potential plugins
12+
over time, as we eventually plan to remove the backcompat apps.
13+
"""
14+
return [
15+
"openedx_learning.apps.openedx_content",
16+
"openedx_learning.apps.openedx_content.backcompat.backup_restore",
17+
"openedx_learning.apps.openedx_content.backcompat.collections",
18+
"openedx_learning.apps.openedx_content.backcompat.components",
19+
"openedx_learning.apps.openedx_content.backcompat.contents",
20+
"openedx_learning.apps.openedx_content.backcompat.publishing",
21+
"openedx_learning.apps.openedx_content.backcompat.sections",
22+
"openedx_learning.apps.openedx_content.backcompat.subsections",
23+
"openedx_learning.apps.openedx_content.backcompat.units",
24+
]

openedx_learning/apps/authoring/components/apps.py

Lines changed: 0 additions & 24 deletions
This file was deleted.

0 commit comments

Comments
 (0)