openedx / openedx/openedx-platform
Certificates tab javascript constructor always fails in Instructor page when it is enabled
Nobody has claimed this yet.
- Dominant language
- Python
- Stars
- 8.2k
- Forks
- 4.4k
- Avg merge
- 6d 18h
- Merged PRs (30d)
- 42
Description
Conditions
There is at least one bug when the instructor page is opened in lms and the Certificates sub tab is enabled.
The conditions required to trigger the bug are:
- Subtab certificates enabled[^1].
FEATURES.CERTIFICATES_INSTRUCTOR_GENERATIONinfalsevalue (default).FEATURES.CERTIFICATES_INSTRUCTOR_GENERATIONintruevalue and additional course settings about certificates.
Trace
On load, the Instructor page run a script which initialize each subtab defined in a hardcoded array named sectionsToInitialize.
Then, each subtab is initialized through its own constructor:
https://github.com/openedx/edx-platform/blob/7f7cade1310db6b50b2e32914b9d8f4d431d69f4/lms/static/js/instructor_dashboard/instructor_dashboard.js#L222-L226
The certificates constructor has a call to initialize the instructor task monitor:https://github.com/openedx/edx-platform/blob/8b4adaf4bd72acb0ea4641dea1b47f2efa28003b/lms/static/js/instructor_dashboard/certificates.js#L111-L114 defined in utils.
In this PendingInstructorTasks function, there are three div containers searched and defined to further implement the running_tasks section:
https://github.com/openedx/edx-platform/blob/8b4adaf4bd72acb0ea4641dea1b47f2efa28003b/lms/static/js/instructor_dashboard/util.js#L371-L373
findAndAssert is a utility function which throw an exception when the selector passed as a parameter has found zero or multiple objects:
https://github.com/openedx/edx-platform/blob/8b4adaf4bd72acb0ea4641dea1b47f2efa28003b/lms/static/js/instructor_dashboard/util.js#L32-L33
Errors
First
The first container searched is not shown when the settings variable FEATURES.CERTIFICATES_INSTRUCTOR_GENERATION is false (see
https://github.com/openedx/edx-platform/blob/master/lms/djangoapps/instructor/views/instructor_dashboard.py#L362,
https://github.com/openedx/edx-platform/blob/master/lms/djangoapps/instructor/views/instructor_dashboard.py#L375 and https://github.com/openedx/edx-platform/blob/master/lms/templates/instructor/instructor_dashboard_2/certificates.html#L82),
which is its default state. As a consequence, the function findAndAssert will always fail for the selector .running-tasks-section interrupting further subtab initialization.
Second
If FEATURES.CERTIFICATES_INSTRUCTOR_GENERATION is defined true through settings, but at least one of enabled_for_course[^2] or html_cert_enabled[^3] is false, then the function findAndAssert will always fail because the mentioned section will be absent.
These additional conditions were added in this commit.
Third
If the previous conditions are met in order to show the .running-tasks-section container, then the third findAndAssert call will fail because there is a second .no-pending-tasks-message container added for the certificate-exception-section.
This section was added in this commit. Apparently, that second message container is useless.
Remediaton suggested
To remediate the bug, we suggest to conditionate the initialization of the running task monitor to the presence of the .running-tasks-section container (not always present as we saw). This means that the first findAndAssert call should be changed for a test to detect the existence of the section before checking the associated inner containers needed for the monitor, and as a precondition to initialize the running tasks monitor.
Additionally, the selectors for inner containers should add the scope of the section to avoid the match to other sections as the certificate-exception section does.
[^1]: To enable the certificate sub tab, go to https://lms-url/admin/certificates/certificategenerationconfiguration and create a new configuration object with the checkbox selected.
[^2]: To make enabled_for_course state true a certificategenerationcoursesetting object needs to be created in association with the course at https://lms-url/admin/certificates/certificategenerationcoursesetting/.
[^3]: In the case of html_cert_enabled a html template needs to be created associated with the course at https://lms-url/admin/certificates/certificatetemplate/.
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start with lms/static/js/instructor_dashboard/instructor_dashboard.js and the certificates constructor, then read PendingInstructorTasks and findAndAssert in lms/static/js/instructor_dashboard/certificates.js and util.js. Check the certificates template and the listed instructor view conditions to cover each feature state. Done means the Certificates subtab initializes without exceptions when the running-tasks section is absent, and existing task-monitor elements are scoped correctly when it is present.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- javascript
- Domain
- frontend
- Issue type
- Bug
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Stale
- Clarity
- Clearly specified
- Newbie friendliness
- 48/100