Skip to content

use courseware_url from language_options, filter them by is_enrollable#3276

Merged
gumaerc merged 8 commits into
mainfrom
cg/dashboard-translations-ui-followup
May 1, 2026
Merged

use courseware_url from language_options, filter them by is_enrollable#3276
gumaerc merged 8 commits into
mainfrom
cg/dashboard-translations-ui-followup

Conversation

@gumaerc
Copy link
Copy Markdown
Contributor

@gumaerc gumaerc commented May 1, 2026

What are the relevant tickets?

Description (What does it do?)

Screenshots (if appropriate):

  • Desktop screenshots
  • Mobile width screenshots

How can this be tested?

Additional Context

…ns course runs by is_enrollable

Co-authored-by: Copilot <copilot@github.com>
Copilot AI review requested due to automatic review settings May 1, 2026 16:59
@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 1, 2026

OpenAPI Changes

No changes detected

View full changelog

Unexpected changes? Ensure your branch is up-to-date with main (consider rebasing).

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Updates the dashboard/program language selection logic to only consider enrollable course runs and removes the previously supported “synthetic” (language option without a concrete V2 run) resolution path, alongside bumping the MITxOnline API client dependency source.

Changes:

  • Filter language_options down to those backed by courseruns with is_enrollable=true, and adjust selection heuristics accordingly.
  • Remove synthetic run creation in getResolvedRunForSelectedLanguage (now returns null when no concrete run/enrollment exists).
  • Update/extend unit tests and switch @mitodl/mitxonline-api-axios dependency to a GitHub-hosted tarball URL.

Reviewed changes

Copilot reviewed 5 out of 6 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
frontends/main/src/app-pages/DashboardPage/CoursewareDisplay/languageOptions.ts Filters language options to enrollable runs; removes synthetic run resolution path.
frontends/main/src/app-pages/DashboardPage/CoursewareDisplay/languageOptions.test.ts Updates fixtures and adds coverage for unenrollable options being ignored.
frontends/main/src/app-pages/DashboardPage/CoursewareDisplay/EnrollmentDisplay.test.tsx Updates test data to include language_options/courseware_url and ensure runs are enrollable where needed.
frontends/main/package.json Points @mitodl/mitxonline-api-axios to a GitHub tarball URL.
frontends/api/package.json Points @mitodl/mitxonline-api-axios to a GitHub tarball URL.
yarn.lock Updates resolution/checksum for the GitHub tarball dependency.

Comment thread frontends/main/package.json Outdated
"@floating-ui/react": "^0.27.16",
"@mitodl/course-search-utils": "^3.5.2",
"@mitodl/mitxonline-api-axios": "2026.4.29",
"@mitodl/mitxonline-api-axios": "https://github.com/mitodl/mitxonline-api-clients/raw/dfa6c8772184ca05c36fda8b745c5e4663310ece/src/typescript/mitxonline-api-axios/package.tgz",
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Copilot doesn't understand that this is temporary

Comment thread frontends/api/package.json Outdated
},
"dependencies": {
"@mitodl/mitxonline-api-axios": "2026.4.29",
"@mitodl/mitxonline-api-axios": "https://github.com/mitodl/mitxonline-api-clients/raw/dfa6c8772184ca05c36fda8b745c5e4663310ece/src/typescript/mitxonline-api-axios/package.tgz",
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Copilot doesn't understand that this is temporary

Comment on lines +278 to +284
run_tag: selectedLanguageOption.run_tag,
__synthetic: true,
} satisfies SyntheticCourseRunV2
return null
Comment on lines -262 to -271
// Return a synthetic selected-language run id/title/courseware mapped onto a
// scoped template run so unenrolled language selection can still resolve.
return {
...templateRun,
id: selectedLanguageOption.id,
title: selectedLanguageOption.title,
courseware_id: selectedLanguageOption.courseware_id,
run_tag: selectedLanguageOption.run_tag,
__synthetic: true,
} satisfies SyntheticCourseRunV2
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

i think we still need this, just include courseware_url via selectedLanguageOption ?

…l because it's still necessary

Co-authored-by: Copilot <copilot@github.com>
Copy link
Copy Markdown
Contributor

@ChristopherChudzicki ChristopherChudzicki left a comment

Choose a reason for hiding this comment

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

👍 working now with getResolvedRunForSelectedLanguage synthetic branch restored.

Comment on lines -140 to -145
if (!selectedLanguageKey) {
const resolvedLanguageKey =
selectedLanguageKey || getDefaultLanguageOptionKey(course) || ""

if (!resolvedLanguageKey) {
return null
}
return (
(course.language_options ?? []).find(
(option) => getLanguageOptionKey(option) === selectedLanguageKey,
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Bug: getDefaultLanguageOptionKey can return null when the default run is unenrollable and its courseware_id differs from the enrollable alternative, causing incorrect language pre-selection.
Severity: MEDIUM

Suggested Fix

In getDefaultLanguageOptionKey, after attempting to find a match by courseware_id, add a fallback step. If no match is found, use the language code from the unenrollable defaultRun to find any enrollable option with the same language code in enrollableLanguageOptions. This ensures a valid key is returned.

Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.

Location:
frontends/main/src/app-pages/DashboardPage/CoursewareDisplay/languageOptions.ts#L120-L145

Potential issue: The function `getDefaultLanguageOptionKey` can incorrectly return
`null` when a user's default course run (`next_run_id`) is unenrollable and the
corresponding enrollable run for the same language has a different `courseware_id`. The
logic fails to fall back to matching by language code. This `null` value causes
`getDistinctLanguageOptions` to miscalculate language frequencies, potentially sorting a
non-default language first. Consequently, UI components like `EnrollmentDisplay.tsx` and
`ContractContent.tsx` may pre-select the wrong language for the user, showing Spanish
instead of English, for example.

Also affects:

  • frontends/main/src/app-pages/DashboardPage/EnrollmentDisplay/EnrollmentDisplay.tsx
  • frontends/main/src/app-pages/DashboardPage/ContractContent/ContractContent.tsx

Did we get this right? 👍 / 👎 to inform future reviews.

@gumaerc gumaerc merged commit dedb0fb into main May 1, 2026
14 checks passed
@gumaerc gumaerc deleted the cg/dashboard-translations-ui-followup branch May 1, 2026 19:21
@odlbot odlbot mentioned this pull request May 1, 2026
3 tasks
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.

3 participants