Skip to content

Admin Loading Issues Fix Pt.2#1130

Open
taheralfayad wants to merge 45 commits intodevfrom
issue/1089-admin_page_loading_issues_2
Open

Admin Loading Issues Fix Pt.2#1130
taheralfayad wants to merge 45 commits intodevfrom
issue/1089-admin_page_loading_issues_2

Conversation

@taheralfayad
Copy link
Copy Markdown
Contributor

@taheralfayad taheralfayad commented Mar 13, 2026

This PR seeks to address the issues with loading courses on the admin panel.

It modifies the courses table by adding the following fields:

  • course_professors: A JSON field to store the names of professors that teach the course

and modifies pre-existing data by:

  • Adding a unique constraint to the composite of lms_course_id and institution_id for courses
  • Updating the columns of lms_account_id and lms_term_id to become foreign keys to the Account and Term tables respectively.

Steps to perform migration:

1.) Provided that you have already run the Version20260304160904 migration, ensure that you have already populated the Account and Term tables. You can populate the Account and Term tables using the make command make admin-panel-retrieve-data TABLES="accounts terms".

2.) Once you've added the Account and Term tables, you're now ready to run the migration in this pull request. Start off by identifying any lms_course_id duplicates in your table. This migration WILL NOT work if you have courses with duplicate lms_course_ids. After you've deleted those courses from the database, go ahead and run the migration Version2026031220211 using make migrate-to VERSION={2026031220214}. The migration should run smoothly from there.

3.) Activate the ETL task that populates the courses table by running make admin-panel-retrieve-data TABLES="courses", and you should now have in your database all the data necessary to power the admin panel.

@taheralfayad taheralfayad requested review from Ishfaq-code, dmols and mbusch3 and removed request for Ishfaq-code March 27, 2026 21:29
@taheralfayad
Copy link
Copy Markdown
Contributor Author

@dmols @mbusch3 I believe this PR is ready for an initial review. There are still features (such as the View report for all courses and the Scan course content) in the admin panel that require some work, but this should at least get the admin panel in a 90% working state.

Copy link
Copy Markdown
Contributor

@Ishfaq-code Ishfaq-code left a comment

Choose a reason for hiding this comment

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

The setup seems to be working fine of going into admin. Some rendering issues when variables are undefined on the frontend which I commented.

Only issue I faced when I scanned one of the courses it did appear to catch that but the stats do not show up on the courses table

Comment thread assets/js/Components/Admin/AdminFilters.js Outdated
@taheralfayad taheralfayad changed the title WIP: Admin Loading Issues Fix Pt.2 Admin Loading Issues Fix Pt.2 Apr 2, 2026
Copy link
Copy Markdown
Contributor

@dmols dmols left a comment

Choose a reason for hiding this comment

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

Changes look good! Admin panel works as expected. Good work!

Copy link
Copy Markdown
Contributor

@Ishfaq-code Ishfaq-code left a comment

Choose a reason for hiding this comment

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

LGTM

@mbusch3
Copy link
Copy Markdown
Contributor

mbusch3 commented Apr 9, 2026

Still to-do:

  • Update migration instructions
  • Remove the long list of professors from the report when it's a "View Report for All Courses". It's unhelpful.
  • Add the account and term filters to the main dashboard... That dashboard reflects data only from the selected term and account, so it makes sense to show it there, too.
  • Update the columns on the course table to the following:
    • Course Name, Instructors, [remove account name... it's in the filter above], Last Scanned, Known Barriers, Potential Barriers, [remove Suggestions], Total Files, Barriers Resolved, and Files Reviewed.

There's more after that, but mostly just prettification of the dashboard, and can be part of a separate PR.

@taheralfayad
Copy link
Copy Markdown
Contributor Author

taheralfayad commented Apr 9, 2026

List of completed to-dos:

  • Remove the long list of professors from the report when it's a "View Report for All Courses". It's unhelpful.
  • Add the account and term filters to the main dashboard... That dashboard reflects data only from the selected term and account, so it makes sense to show it there, too.
  • Update the columns on the course table to the following:
    • Course Name, Instructors, [remove account name... it's in the filter above], Last Scanned, Known Barriers, Potential Barriers, [remove Suggestions], Total Files, Barriers Resolved, and Files Reviewed.

Comment thread Makefile
Comment thread assets/js/Components/Admin/AdminDashboard.js
Comment thread assets/js/Components/Admin/CoursesPage.js
Comment thread assets/js/Components/Reports/ReportsTable.js
@taheralfayad
Copy link
Copy Markdown
Contributor Author

Regarding this:

  • Update migration instructions

@mbusch3 and I have had discussions regarding how best to roll these changes out in production for UCF, as well as for the open-source project. At the moment, the main hindrance to making this update fully automatic is that there might be rows in the Course table that contain duplicate lms_course_id and insitution_id pairs, which violates the joint uniqueness constraint that I added on lms_course_id and institution_id pairs. This is due to an unresolved bug where courses titled New Course # are added to the database on LTI launch. I can add a line in the migration to automatically delete rows in the Course table that contain the string "New Course", but I personally favored manual remediation just to ensure minimal data loss. I would also recommend that we address the New Course # issue prior to merging these changes. Any thoughts?

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.

4 participants