WIP Proposal: Unified authoring app

[ormsbee]

This is a wacky proposal that I've been kicking around in my head since I started working seriously on #452, and ran into issues with renaming the app and/or moving models around.

What is this?

This PR refactors the repo to combine all authoring apps (publishing, components, content, collections, etc.) into a single authoring app from Django's point of view. But the boundaries previously set up by the apps still exist in openedx_learning.apps.authoring.applets.* which has different packages for components, collections, and all the rest.

Each of these sub-apps still have their own models.py and admin.py files, though these are all stitched together by the higher level authoring files. So for instance, openedx.apps.authoring.models imports everything from the sub-apps. We could make utility wrappers to make this more convenient later. (Edit: Introspection magic breaks code autocomplete.)

It would look like:

authoring
├── __init__.py
├── admin.py
├── apps.py
├── management
│   ├── __init__.py
│   └── commands
│       ├── __init__.py
│       ├── add_assets_to_component.py
│       ├── lp_dump.py
│       └── lp_load.py
├── migrations
│   ├── 0001_initial.py
│   ├── 0002_rename_tables_to_oel_authoring.py
│   └── __init__.py
├── models.py
└── applets
    ├── __init__.py
    ├── backup_restore
    │   ├── __init__.py
    │   ├── admin.py
    │   ├── api.py
    │   ├── models.py
    │   ├── serializers.py
    │   ├── toml.py
    │   └── zipper.py
    ├── collections
    │   ├── __init__.py
    │   ├── admin.py
    │   ├── api.py
    │   ├── models.py
    │   └── readme.rst
    ├── components
    │   ├── __init__.py
    │   ├── admin.py
    │   ├── api.py
    │   ├── models.py
    │   └── readme.rst
    ├── contents
    │   ├── __init__.py
    │   ├── admin.py
    │   ├── api.py
    │   └── models.py
    ├── publishing
    │   ├── __init__.py
    │   ├── admin.py
    │   ├── api.py
    │   ├── contextmanagers.py
    │   ├── models
    │   │   ├── __init__.py
    │   │   ├── container.py
    │   │   ├── draft_log.py
    │   │   ├── entity_list.py
    │   │   ├── learning_package.py
    │   │   ├── publish_log.py
    │   │   └── publishable_entity.py
    │   └── readme.rst
    ├── sections
    │   ├── __init__.py
    │   ├── admin.py
    │   ├── api.py
    │   └── models.py
    ├── subsections
    │   ├── __init__.py
    │   ├── admin.py
    │   ├── api.py
    │   └── models.py
    └── units
        ├── __init__.py
        ├── admin.py
        ├── api.py
        └── models.py

Why do this?

I still believe that having small, discrete app-like things is useful for controlling complexity, and I don't want to give that up. That being said, refactoring is made much harder when we want to try to either change the names of real Django apps (e.g. contents to media) or if we want to move models around (e.g. Container/ContainerVersion leaving the publishing app to go to a new containers app). Having all the models be in one namespace will make shifting the boundaries between them much easier.

This will have a few other minor benefits. We can do a top level api.py file for authoring in a way that's consistent with other apps in the system. It sort of sets up the umbrella authoring app as the holder of the public interface. It also makes it less cumbersome to enter in the list of apps.

On the downside, there's less consistency in terms of what goes where. Management commands, migrations, and app initialization code has to go in the root authoring app.

How is it working?

The other apps disappear entirely, and are replaced by one authoring app. The authoring app's initial migration tries to be smart–it will create a whole new set of tables if it's a new database, but if it detects an up-to-date Ulmo install of app migrations, it will take over the model state for all those models without running any database operations. (The second migration then changes all the table names to start with oel_authoring.) Being just post-release is a perfect time to do this pretty radical realignment.

What to call it?

I'm not sure what to call this practice. I'm currently going with authoring being an "umbrella app", and the small things being "applets"... but I'm open to suggestions. In DDD terms, the authoring app would be a subdomain, and the individual applets would be bounded contexts—but "Context" already means something in Django, so I thought it would be too confusing to borrow that terminology.

If we're just moving things around, why are there so many more lines removed than added?

It's because we're deleting app.py and migration files for the individual apps.

[ormsbee]

After this transition, I think I'd want to eventually refactor the applets package to look something more like:

applets/
        collections/
        components/
        containers/
                   sections/
                   subsections/
                   units/
        media/
        publishing/

[ormsbee]

I'm killing this auto-discovery code from the PR because I don't think the convenience is worth the loss of code introspection in editors and CI, but I want to preserve it here in case we need to write some applet-traversing code again:

"""
This was an attempt to make cute and clever code to dynamically discover and
import all applet modules of a certain type for aggregation purposes (e.g. the
authoring/models.py file importing from all applet models.py files). This code
actually does work, but I only realized after buildling it that it breaks
editor introspection/autocomplete, which makes the cost far too high for this
convenience.
"""

import functools
import importlib
import inspect
import pathlib


def auto_import(module_name):
    """Auto-import all modules with a given name in subdirs of applets."""
    caller_frame_info = inspect.stack()[1]
    caller_module = inspect.getmodule(caller_frame_info[0])
    caller_module_name = caller_module.__name__

    # converts openedx_learning.authoring.models -> openedx_learning.authoring
    import_base = ".".join(caller_module_name.split(".")[:-1])

    caller_filepath = caller_frame_info.filename
    caller_dir = pathlib.Path(caller_filepath).resolve().parent
    applets_dir = caller_dir / "applets"
    module_paths = applets_dir.rglob(f"{module_name}.py")
    relative_paths = [
        module_path.relative_to(caller_dir) for module_path in module_paths
    ]
    all_modules_dict = {}
    for relative_path in sorted(relative_paths):
        module_import_name = f"{import_base}." + ".".join(relative_path.parts)[:-3]
        module = importlib.import_module(module_import_name)
        module_dict = vars(module)
        if '__all__' in module_dict:
            module_dict_to_add = {
                key: module_dict[key]
                for key in module_dict['__all__']
            }
        else:
            module_dict_to_add = {
                key: val
                for (key, val) in module_dict.items()
                if not key.startswith('_')
            }
        all_modules_dict.update(module_dict_to_add)

    return all_modules_dict


auto_import_models = functools.partial(auto_import, "models")
auto_import_api = functools.partial(auto_import, "api")
auto_import_admin = functools.partial(auto_import, "admin")

[bradenmacdonald]

The authoring app's initial migration tries to be smart–it will create a whole new set of tables if it's a new database, but if it detects an up-to-date Ulmo install of app migrations, it will take over the model state for all those models without running any database operations.

Could this be done using Django's "squashed migration" feature instead of custom logic?

I'm currently going with authoring being an "umbrella app", and the small things being "applets"... but I'm open to suggestions.

I'm not a fan of "applet", so I'd suggest "[sub]modules". Or perhaps better, continue to call them "apps" and just rename the larger thing - "umbrella app", "super-app", "domain", etc.

[ormsbee]

I'm not a fan of "applet", so I'd suggest "[sub]modules". Or perhaps better, continue to call them "apps" and just rename the larger thing - "umbrella app", "super-app", "domain", etc.

My first stab at this was to shove them into a modules sub-directory, and then I was briefly floating the idea of calling authoring a "modular app". I backed away from it because I thought that overloading the use of the word "module" was bad, given its common Python meaning.

For the same reason, I don't want to call the small things "apps" because "apps" really means something in Django, and I don't want to overload that term. One of the reasons I went for "applet" was because it did not collide with any existing terminology that I know of in the Python/Django sphere. (I mean yes, there's the Java connotation, but Java applets have been dead for almost a decade now, and they had waned in popularity years before that.)

In our meeting, "subapps" was suggested, which I'm okay with. I honestly still prefer "applets" though, because I think it more clearly conveys that these things are not actually real Django apps, but instead are more diminutive, app-like things. If I see a subapps package, it still makes me think that the things inside it are actual Django apps.

[ormsbee]

@bradenmacdonald: I may be missing something, but I don't really see how to make squashed migrations work without keeping the shells of the old apps around in their old locations (at least enough for a apps.py and migrations folder), and that seems like it would be more confusing than it's worth. If the squashed migration happens only in the authoring app, it also doesn't work because deciding whether something is migrated or not doesn't depend on the state of the schema but on the entries in the django_migrations table, and that table will have no entries for oel_authoring regardless.

FWIW, the schema changes were auto-generated in the sense that I generated them from looking at the current state of the model files and then indented them in one level to wrap them in my SeparateDatabaseAndState subclass.

[bradenmacdonald]

keeping the shells of the old apps around in their old locations (at least enough for a apps.py and migrations folder)

I see. I guess that's the price that we'd have to pay to allow migrations from any point in time rather than Ulmo exactly.

[ormsbee]

I see. I guess that's the price that we'd have to pay to allow migrations from any point in time rather than Ulmo exactly.

Actually, I may have an idea around this. The two important things are the labels and getting all the apps into edx-platform's INSTALLED_APPS. I think I can shove the migration remnants into a corner somewhere, make a function to return the app locations that edx-platform needs, and hook up the migrations to be sequential so this all works out okay.