feat: Add language support in courses #3335

arslanashraf7 · 2024-12-12T13:28:57Z

What are the relevant tickets?

https://github.com/mitodl/hq/issues/6239

Description (What does it do?)

Add language support for courses and programs
Adds a new model CourseLanguage to manage supported languages, The model has priority field that defines the sorting order of course by language on Catalog
Also, updates the external sync API command and task to add/update the languages as received from the external APIs (Emeritus, Global Alumni atm). if there is no language key available in the API the default language would be set to English
Prefills the language model with currently supported languages. Supported languages list contains English, Spanish, Portuguese, Mandarin, Italian, French as per https://github.com/mitodl/hq/issues/6111
Pre-fills the courseware pages with languages (Sets English to all the existing pages), If a course has different language from the external API that would be synced accordingly
Adds the language key in the courses & programs APIs
Updates the seed data commands to add default language as well

Screenshots (if appropriate):

TBA

Desktop screenshots
Mobile width screenshots

Manage Languages

Assign languages (CMS)

How can this be tested?

Testing language Model & data Prefill

Setup a couple of courses on the master branch
Now set up on this branch to run migrations, a New table Course Languages should now be available in Django Admin with pre-filled languages as mentioned above. The English language would have the priority set to 1 whereas the other languages should have 100. This is because we want to sort the courses by English language for now. However, this priority key can be used to test different languages with different priority values. 1 means highest priority
Test that the language name is case insensitive, which means there should only be one language with a name in the system
Once, you have tested the above, Open any Course/Program pages in CMS and they should all have a language field (Set to English by default)
A language object should be protected, e.g. if a language is associated with a course page it should not be deleted

Testing External APIs

Once you have tested above, you should run the command sync_external_course_runs --vendor-name emeritus/Global Alunmni
Test that the course pages from the extern vendors have updated language acquired by the external API set to them
If there is a language in the external API that was not pre-filled, it should now be created automatically after running the command
This has two test cases, 1) when the command was already run and data was present and you run the command 2) When command is run for the first time. In both cases the course page languages should be updated accordingly

Testing internal APIs

Test that api/courses & /api/programs have a language field in the JSON response

Testing Catalog Course sorting

Currently, the courses on the catalog are sorted by next run date, but after this PR they should be sorted first by language priority and then the next run date (This behavior is only for default sorting)
If a different sort is selected on Catalog, then courses/programs should not be sorted on language priority

Testing Seed Data

Test seed data command, it should create course pages with the language English set to the by default

There are no frontend-level changes for now, except for the ones mentioned above

Additional Context

asadali145

Looks good overall. A couple of small suggestions +
We should use select_related in the API Views and Catalog to avoid n+1 queries for the language table.

Also, I haven't noticed any discussion/comment but It would be a good idea to display the course language in the Catalog Card or a card in the product detail page. I know these will be done in a follow up but just wanted to leave my thoughts here for reference.

asadali145 · 2025-01-09T08:14:23Z

cms/models.py

+        null=False,
+        blank=False,


nit: null and blank default to False, can be removed.

These flags were added and then changed in the same PR and this thing has 2 migrations so I intentionally kept it like this to keep readability

asadali145 · 2025-01-09T08:25:28Z

courses/sync_external_courses/external_course_sync_api_test.py

+    if not has_language:
+        assert external_course_page.language.name == "English"


I am not able to understand this test condition.

This was added to test the case where if there is no language key available from the external APIs the default English language should be assigned to the course. But, As discussed I'll add the else condition to check for provided language and I'll add a code comment above for more readability

asadali145 · 2025-01-09T08:43:23Z

courses/models.py

+
+    class Meta:
+        constraints = [
+            models.UniqueConstraint(Lower("name"), name="unique_language_name")


nit: can provide a better message by passing violation_error_message. Otherwise, it will display a less user-friendly message Constraint “unique_language_name” is violated..

arslanashraf7 · 2025-01-09T13:00:10Z

@asadali145 I've addressed the feedback in 7a8e3aa. Let me know if all the changes look good to you.

asadali145

Neatly done, Looks GooD!

asadali145 · 2025-01-09T14:38:35Z

courses/views/v1/__init__.py

+            "platform",
+            "programpage__language",
+            "externalprogrampage__language",
+        )
        .prefetch_related(courses_prefetch, products_prefetch)


Just noticed we need to select_related the course language for the program courses.

odlbot deployed to xpro-ci-pr-3335 December 12, 2024 13:31 View deployment

arslanashraf7 force-pushed the arslan/6239-add-language-support branch 4 times, most recently from 91c897f to 70a3859 Compare January 8, 2025 10:53

arslanashraf7 marked this pull request as ready for review January 8, 2025 13:15

arslanashraf7 force-pushed the arslan/6239-add-language-support branch from b14ab48 to 0085b1c Compare January 8, 2025 19:19

arslanashraf7 added Needs Review and removed Needs Review labels Jan 8, 2025

feat: Add language support for courses

8025f03

arslanashraf7 force-pushed the arslan/6239-add-language-support branch from 0085b1c to 8025f03 Compare January 9, 2025 07:38

arslanashraf7 added the Needs Review label Jan 9, 2025

asadali145 self-assigned this Jan 9, 2025

asadali145 requested changes Jan 9, 2025

View reviewed changes

asadali145 added Waiting on Author and removed Needs Review labels Jan 9, 2025

address feedback

7a8e3aa

arslanashraf7 force-pushed the arslan/6239-add-language-support branch from bcdb271 to 7a8e3aa Compare January 9, 2025 12:44

arslanashraf7 requested a review from asadali145 January 9, 2025 13:00

arslanashraf7 added Needs Review and removed Waiting on Author labels Jan 9, 2025

asadali145 approved these changes Jan 9, 2025

View reviewed changes

asadali145 added Waiting on Author and removed Needs Review labels Jan 9, 2025

asadali145 reviewed Jan 9, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Add language support in courses #3335

feat: Add language support in courses #3335

arslanashraf7 commented Dec 12, 2024 •

edited

Loading

asadali145 left a comment

asadali145 Jan 9, 2025

arslanashraf7 Jan 9, 2025

asadali145 Jan 9, 2025

arslanashraf7 Jan 9, 2025 •

edited

Loading

asadali145 Jan 9, 2025

arslanashraf7 commented Jan 9, 2025

asadali145 left a comment

asadali145 Jan 9, 2025

		if not has_language:
		assert external_course_page.language.name == "English"

feat: Add language support in courses #3335

Are you sure you want to change the base?

feat: Add language support in courses #3335

Conversation

arslanashraf7 commented Dec 12, 2024 • edited Loading

What are the relevant tickets?

Description (What does it do?)

Screenshots (if appropriate):

How can this be tested?

Additional Context

asadali145 left a comment

Choose a reason for hiding this comment

asadali145 Jan 9, 2025

Choose a reason for hiding this comment

arslanashraf7 Jan 9, 2025

Choose a reason for hiding this comment

asadali145 Jan 9, 2025

Choose a reason for hiding this comment

arslanashraf7 Jan 9, 2025 • edited Loading

Choose a reason for hiding this comment

asadali145 Jan 9, 2025

Choose a reason for hiding this comment

arslanashraf7 commented Jan 9, 2025

asadali145 left a comment

Choose a reason for hiding this comment

asadali145 Jan 9, 2025

Choose a reason for hiding this comment

arslanashraf7 commented Dec 12, 2024 •

edited

Loading

arslanashraf7 Jan 9, 2025 •

edited

Loading