Skip to content

Conversation

@kdmccormick
Copy link
Member

@kdmccormick kdmccormick commented Jan 8, 2025

Description

...

Supporting information

This will be the full implementation of:

Currently blocked by:

This is all part of:

Testing instructions

TBD

Deadline

Needs to land by Teak cutoff (Apr 9) at the very very latest.

Other information

TBD

Sandbox env

Link: TBD
UN/PW: openedx / openedx

Settings

TUTOR_GROVE_COMMON_SETTINGS : |
  SEARCH_ENGINE: "openedx.core.djangoapps.content.search.engine.MeilisearchEngine"

PLUGINS:
- mfe
- grove
- meilisearch
- s3
- k8s_harmony
PLUGIN_INDEXES:
- https://overhang.io/tutor/main

Tutor requirements

git+https://github.com/overhangio/tutor.git@main
git+https://github.com/overhangio/tutor-mfe.git@main
git+https://gitlab.com/opencraft/dev/tutor-contrib-grove.git@main
git+https://github.com/hastexo/tutor-contrib-s3.git@main
git+https://github.com/open-craft/openedx-k8s-harmony.git@kaustav/support_tutor_19_new#egg=tutor-contrib-harmony&subdirectory=tutor-contrib-harmony-plugin

@kdmccormick kdmccormick added the create-sandbox open-craft-grove should create a sandbox environment from this PR label Jan 14, 2025
@open-craft-grove
Copy link

Sandbox deployment failed 💥
Please check the settings and requirements.
Retry deployment by pushing a new commit or updating the requirements/settings in the pull request's description.
📜 Failure Logs
ℹ️ Grove Config, Tutor Config, Tutor Requirements

@open-craft-grove
Copy link

Sandbox deployment failed 💥
Please check the settings and requirements.
Retry deployment by pushing a new commit or updating the requirements/settings in the pull request's description.
📜 Failure Logs
ℹ️ Grove Config, Tutor Config, Tutor Requirements

@kdmccormick kdmccormick removed the create-sandbox open-craft-grove should create a sandbox environment from this PR label Feb 25, 2025
@kdmccormick kdmccormick force-pushed the kdmccormick/library-migration branch from 2d66c67 to 0a239e6 Compare February 25, 2025 16:33
@kdmccormick
Copy link
Member Author

kdmccormick commented Mar 14, 2025

@ormsbee , as we've discussed, authors will be able to:

  • Forwarded-migrate, i.e. copy v1 lib into a v2 collection and forward all existing course references to the v2 blocks. This can be done exactly once
  • Copy-migrate, i.e. copy v1 lib into a v2 collection but with no forwarding. This can be done as many times as they like.

I'm thinking about how to encode both of these kinds of migrations. Here are three ideas, none of which I love. (In each idea, there is a LegacyLibrary(Forwarded|Copy)BlockMigration model hanging off for each block).

1. No data model for copy-migrate

class LegacyLibraryForwardedMigration(Model):
    source_key = LearningContextKeyField(unique=True)
    target_library = ForeignKey(ContentLibrary)
    target_collection = ForeignKey(Collection, null=True, unique=True)
    migrated_by = ...
    migrated_at = ...

# Copy-migrate events are not recorded in the DB

2. Shared model for migrate-with-forwarding and copy-migrate

class LegacyLibraryMigration(Model):
    source_key = LearningContextKeyField()   # should be unique iff `forward==True`
    forwarded = BooleanField() 
    target_library = ForeignKey(ContentLibrary)
    target_collection = ForeignKey(Collection, null=True, unique=True)
    migrated_by = ...
    migrated_at = ...

3. Two different data models

class LegacyLibraryForwardedMigration(Model):
    source_key = LearningContextKeyField(unique=True)
    target_library = ForeignKey(ContentLibrary)
    target_collection = ForeignKey(Collection, null=True, unique=True)
    migrated_by = ...
    migrated_at = ...
class LegacyLibraryCopyMigration(Model):
    source_key = LearningContextKeyField()
    target_library = ForeignKey(ContentLibrary)
    target_collection = ForeignKey(Collection, null=True, unique=True)
    migrated_by = ...
    migrated_at = ...

What I want to do (but don't know how to do in Django)

class LegacyLibraryMigration(Model):
    source_key = LearningContextKeyField()
    target_library = ForeignKey(ContentLibrary)
    target_collection = ForeignKey(Collection, null=True, unique=True)
    migrated_by = ...
    migrated_at = ...
class LegacyLibraryMigrationEnableForwarding(Model):
    migration = OneToOneField(LegacyLibraryMigration)
    source_key = LearningContextKeyField(unique=True)
    class Meta:
        ensure_equal("source_key", "migration.source_key")  # <-- Fake Django. Is this possible?

@kdmccormick kdmccormick force-pushed the kdmccormick/library-migration branch from 0a239e6 to 093dd01 Compare March 14, 2025 22:01
@kdmccormick
Copy link
Member Author

^ We talked through this in the latest libraries sync, and ended up deciding to invert the relationship between the two models. That is: there is a LegacyLibraryMigrationSource row for each legacy library, which is pointed to by many LegacyLibraryMigrations, but which points at a single one of them as the authoritative_migration.

@kdmccormick kdmccormick force-pushed the kdmccormick/library-migration branch from 093dd01 to 1400803 Compare March 18, 2025 21:36
target_collection = models.ForeignKey(
Collection,
unique=True, # Any given collection should be the target of at most one V1 library migration.
on_delete=models.SET_NULL, # Collections can be deleted, but the migrated blocks (and the migration) survive.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FWIW, Collections aren't really removed, just soft-deleted. So if you wanted to, you could remove the target_library as a separate field, and just rely on this relation to get you to the correct library.

@feanil
Copy link
Contributor

feanil commented Jan 9, 2026

@kdmccormick is this PR still relevant, it looks like an early POC of work that has landed?

@kdmccormick
Copy link
Member Author

Nope, it's obsolete now, thanks!

@kdmccormick kdmccormick closed this Jan 9, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants