Initial variant support#3605
Open
jkachel wants to merge 5 commits into
Open
Conversation
…s; added fields and migration; added to API output and included for filtering in v2/courses more to do and no tests, but good stopping point
… other apps); add supported variant tracking table; add variant options mixin, update CourseRun to use that
… v2 serializer, regen the spec, few other ancillary fixes
…st prefetch according to B2B options; add migration to add default variant; fix some other issues that popped up
…ut a list of the variant runs for a given course
OpenAPI ChangesShow/hide ## Changes for v0.yaml: |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This is pretty WIP but getting this out so people can build off the API and do some testing.
What are the relevant tickets?
mitodl/hq#11401
Description (What does it do?)
Gets variant support built out for courses and runs.
Courses get a "possible variants" field that tells you what the course should have. This is exposed in the
v2/coursesAPI. A list of variant runs isn't in here; this is covered later.Course runs get a couple new fields,
variant_lengthandvariant_industry, which are enums. Thev2/coursesAPI includes filtering for these like it does for language. (These fields are wrapped up in a mixin so that they can be put elsewhere and all this can be consistent.)There's a new API in
v3/courses/variants/that takes a list of course IDs, a contract ID, and the variant options. Assuming the user is in the contract, this will return the runs that match the request or the default for each course.How can this be tested?
The migrations set up a default set of variant options. The default is
language = "en", industry = "", length = "".You can add more variants through the Course page in the Django Admin. There's a new inline that should work for this. You can also set the variants in the Course Runs in Django Admin too. They're at the top of the page because the fields come from a mixin now (including language; I will fix this but it's not fixed now).
You will need courses in a B2B contract to test the variant stuff. This is technically B2B only other than the languages. The language fields that were there in the API are still there for that reason.
When you hit the
v2/coursesAPI, you should see a list of the possible variants that the course should support. You can then feed that into thev3/courses/variants/API and get back the appropriate course runs for a given contract. See below for notes/questions.Additional Context
As noted this is pretty WIP, there are tests failing and other tests need to be written, and the management commands will need some work too. But I don't think the API will change, unless someone needs me to change something about it.
I added the language validation stuff in for this - digging through the translation effort, it looks like we are tagging the runs using the language codes from openedx-translations. These are technically ISO 639-1 two-letter codes plus a locale code (
es-419,de-DE,zh-HK). There's some code in here that decodes those using a lookup table and usingpycountryfor the ISO code - we should probably do something similar for how we display these in the frontend (since AFAIK it's just a hard-coded LUT there?).It's really important that you fix up some of your course runs for this stuff to work. The default variant set expects an "en" language course run at least within your given contract, so make sure you have one. Setting the languages on these via a migration or something is actually kind of non-trivial given the constraints we have on the course run model.
Questions: