Epic 11.5 - Import APIs #36389
Epic 11.5 - Import APIs #36389
Conversation
* feat: [AXM-1607] create initial DB layer * refactor: [AXM-1607] resolve lint errors, clean up code & add migrations * refactor: [AXM-1607] simplify raw user_id in admin & improve validation by checking for duplicate keys * refactor: [AXM-1607] lint fixes #2 --------- Co-authored-by: Andrii <[email protected]>
* feat: [AXM-1621] add staged content creation * refactor: [AXM-1621] revert unnecessary changes & functions refactor * feat: [AXM-1621] add CourseToLibraryImport creation & update + refactor to process multiple course ids --------- Co-authored-by: Andrii <[email protected]>
* feat: [AXM-1614] add course to library import feature
openedx-webhooks
added
the
open-source-contribution
|
Thanks for the pull request, @cmltaWt0! This repository is currently maintained by Once you've gone through the following steps feel free to tag them in a comment and let them know that your changes are ready for engineering review. 🔘 Get product approvalIf you haven't already, check this list to see if your contribution needs to go through the product review process.
🔘 Provide contextTo help your reviewers and other members of the community understand the purpose and larger context of your changes, feel free to add as much of the following information to the PR description as you can:
🔘 Get a green buildIf one or more checks are failing, continue working on your changes until this is no longer the case and your build turns green. 🔘 Update the status of your PRYour PR is currently marked as a draft. After completing the steps above, update its status by clicking "Ready for Review", or removing "WIP" from the title, as appropriate. Where can I find more information?If you'd like to get more details on all aspects of the review process for open source pull requests (OSPRs), check out the following resources: When can I expect my changes to be merged?Our goal is to get community contributions seen and reviewed as efficiently as possible. However, the amount of time that it takes to review and merge a PR can vary significantly based on factors such as:
💡 As a result it may take up to several weeks or months to complete a review and merge your PR. |
There was a problem hiding this comment.
Thanks for the prototype Max!
One high-level direction question for @ormsbee about whether to combine these models with my legacy library import models. If he thinks they should stay separate, then @cmltaWt0 I have some data model comments for this PR.
| status = models.CharField( | ||
| max_length=100, | ||
| choices=CourseToLibraryImportStatus.choices, | ||
| default=CourseToLibraryImportStatus.PENDING | ||
| ) |
There was a problem hiding this comment.
We have django-user-tasks for situations where we need to track that status of a Celery task and associate it with a user. Example - CourseExport
Would you be able to leverage that library as the basis for this?
| course_ids = models.TextField( | ||
| blank=False, | ||
| help_text=_('Whitespace-separated list of course keys for which to compute grades.'), | ||
| validators=[validate_course_ids] | ||
| ) |
There was a problem hiding this comment.
I think we should have one course per model instance. That way, this could be a LearningContextKeyField, and we'd have simpler validation and querying of this table. It will also make it easier to manage these tasks in Django admin if each one is associated with a single course.
If it makes this PR easier, you can assume that we are importing one course at a time. If it becomes important, we can implement multi-course import later at the REST API layer, and have a separate task model to track it.
| help_text=_('Whitespace-separated list of course keys for which to compute grades.'), | ||
| validators=[validate_course_ids] | ||
| ) | ||
| library_key = models.CharField(max_length=100) |
There was a problem hiding this comment.
Let's make this a ForeignKey to ContentLibrary (or even LearningPackage). That gets us a few benefits:
- We get more efficient JOINs when querying
- We get database-level validation that we this model points at a valid learning package
| validators=[validate_course_ids] | ||
| ) | ||
| library_key = models.CharField(max_length=100) | ||
| source_type = models.CharField(max_length=30) |
| ) | ||
| library_key = models.CharField(max_length=100) | ||
| source_type = models.CharField(max_length=30) | ||
| metadata = models.JSONField(default=dict, blank=True, null=True) |
| Represents a component version that has been imported into a content library. | ||
| This is a many-to-many relationship between a component version and a course to library import. | ||
| """ | ||
|
|
There was a problem hiding this comment.
Let us also add an FK to Component. That way, we can declare that component and library_import are unique_together.
I believe source_usage_key and library_import can also be declared as unique_together.
In other words, we assert that for any given library import event, the source block is unique, and the destination block is unique.
| return cls.objects.filter(id=import_id).first() | ||
|
|
||
|
|
||
| class ComponentVersionImport(TimeStampedModel): |
There was a problem hiding this comment.
Soon we will need to support container imports as well. When that time comes, were you thinking that we'd have a separate ContainerImport model? Or, should we generalize this model into PublishableEntityImport?
| @@ -0,0 +1,74 @@ | |||
| """ | |||
| Models for the course to library import app. | |||
There was a problem hiding this comment.
I realize that I was quite insistent earlier that this app should explicitly focus on course-to-library imports. As I've worked on the legacy library migration, though, I'm realizing I may have been wrong about that. I see a lot of parallels between these models, and the legacy-lib-to-library models in my PR.
@ormsbee , do you think it would be reasonable to consider this djangoapp as a general import_to_learning_package application? The models would be something like:
class LegacyContentImportStatus(UserTaskStatus): ...
class LegacyContentImportSource(Model):
context_key = LearningContextKeyField(...) # course or legacy lib
authoritative_import = FK(LegacyContentToLearningPackageImport, null=True) # where to forward content to, if applicable
class LegacyContentToLearningPackageImport(Model):
source = FK(LegacyContentImportSource)
import = OneToOneField(LearningPackageImport)
class LearningPackageImport(Model):
target = FK(LearningPackage) # destination lib (or, in the future, learning-core course?)
target_collection = FK(Collection, null=True) # if applicable
class LegacyContentToPublishableEntityImport(Model):
package_migration = FK(LegacyContentToLearningPackageMigration)
source = UsageKeyField()
target = FK(PublishableEntity)
class Meta:
unique_together = [("package_migration", "source"), ("package_migration", "target")]
* feat: [AXM-1726] add container feature for import * refactor: [AXM-1726] post-review changes --------- Co-authored-by: Andrii <[email protected]>
* feat: [AXM-1635] create block importing route
Work in progress - DO NOT MERGE. This PR contributes to EPIC 11.5: Import Course to Library API.
Accompanied ADR: #36380
@NiedielnitsevIvan @ormsbee @kdmccormick FYI