-
Notifications
You must be signed in to change notification settings - Fork 3
Program Summary #2590
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Program Summary #2590
Conversation
42ce4d7
to
0586c1d
Compare
constructor({ id, data }: NodeConstructorOpts = {}) { | ||
this.id = id ?? uniqueNodeId.enforce(faker.number.int) | ||
this.data = { | ||
// @ts-expect-error OpenAPI spec is incorret |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Assuming that improve openapi for req_tree is merged, these @ts-expect-errors
will start triggering errors on the corresponding new version of @mitodl/mitxonline-api-axios
.
I did try a local copy of the client generated from improve openapi for req_tree and no errors are caused except the @ts-expect-error
lines, whose fix is trivial (remove them).
docker-compose.services.yml
Outdated
- MITX_ONLINE_UPSTREAM=${MITX_ONLINE_UPSTREAM:-host.docker.internal:8013} | ||
- MITX_ONLINE_DOMAIN=${MITX_ONLINE_DOMAIN:-host.docker.internal} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This and the related changes should mean we don't need to use local ips (which change when we come into the office...) for MIT Learn <> MITxOnline integration.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The program summary box works for me and looks good vs the Figma design.
I'll leave it up to you as to whether or not to do this, but normally I would separate the local dev Docker hostname changes into a separate PR. Nonetheless, I did notice something that looks to be a typo:
docker-compose.services.yml
Outdated
<<: *host-access | ||
environment: | ||
- CSRF_COOKIE_DOMAIN=${CSRF_COOKIE_DOMAIN:-.odl.local} | ||
- CSRF_COOKIE_DOMAIN=${CSRF_COOKGreatIE_DOMAIN:-.odl.local} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is this about?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
5e4bbc7
to
b01de9b
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 Looks good!
What are the relevant tickets?
Description (What does it do?)
Adds the program summary box to MIT Learn program pages.
Screenshots (if appropriate):
How can this be tested?
Prerequisite: A working mitxonline local installation; does not require OpenEdx integration.
Setup: Connect MITxOnline <--> MIT Learn:
Now:
./manage.py create_courseware
,./manage.py create_courseware_page
; see--help
for details. Note: program readable ids should start withprogram-v1:
../manage.py backpopulate_mitxonline_data --delete
, and./manage.py backpopulate_mitxonline_data
product-page-course
feature flag is on in your personal posthoghttp://learn.odl.local:8062/programs/:program_id