-
Notifications
You must be signed in to change notification settings - Fork 3
better bad data handling for the org dashboard #2427
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
Conversation
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 clean!
|
||
const programsWithCourses = programQueries | ||
.map((query, index) => { | ||
if (!query.data?.results || !query.data.results.length) { |
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.
if (!query.data?.results || !query.data.results.length) { | |
if (!query.data?.results?.length) { |
} | ||
|
||
// Only render if at least one program has courses | ||
if (!hasAnyCourses) { |
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.
Are we likely to ever hit this scenario or this just the bad data defense? Thinking if it's ever expected under normal conditions it's odd to display the program title and description with load skeleton and then to disappear it once loaded with no courses (as opposed to just display e.g. "No courses are available in this program at this time").
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.
It will show the loading skeleton for the program while the API response is still coming though, but if it's determined that the program isn't returning courses, then nothing will be shown. This is mostly a bad data defense, realistically this should never happen in production. I will ask Steve / Bilal what might be expected here when I talk to them next, but just leave it as-is for now.
f4f11a0
to
5596b04
Compare
What are the relevant tickets?
Closes https://github.com/mitodl/hq/issues/8057
Description (What does it do?)
This PR primarily fixes an issue where if you put programs from multiple orgs in a program collection, you would see an infinite loading skeleton for the programs that aren't part of the currently displayed org. This is because the API call to get the program detail filters on
org_id
, but the program collection endpoint returns program ids that are not a part of the given org. This acts as a safegaurd against that, although there should maybe be a PR in MITx Online as well to make sure that programs returned for a collection are also filtered onorg_id
. This PR also adds a message if no programs are found to be associated with the org being viewed.Screenshots (if appropriate):
How can this be tested?
In order to test this, you need a basic installation of
mitxonline
up and running with example data in it. You may be able to skip one or more steps if you have already done them:hosts
redirects for the following domains, replacing the example IP with your local IP address (Google how to get this if unsure, mine is 192.168.1.50)mitxonline
: https://github.com/mitodl/mitxonline.env
file with the following values:mitxonline
withdocker compose up --build -d
docker compose exec web ./manage.py promote_user promote --superuser --email [email protected]
docker compose exec web ./manage.py populate_course_data
pants docs ::
dist/sphinx/index.html
, read the section on generating a B2B organization / contract and create two, adding some of the test courses to both, and some to only org a / only org bProgram
and add some courses to the program that are included in your B2B org, making sure to mark the program as "live"mit-learn
, we need to set some env variables:enrollment-dashboard
andmitlearn-organization-dashboard
feature flags for all usersMIT Learn
[email protected]
test user