Wikipedia Education Program/Database Analysis Notes
Appearance
This page is obsolete. It is being retained for archival purposes. It may document extensions or features that are obsolete and/or no longer supported. Do not rely on the information here being up-to-date. |
Patches
[edit]- Fix summary info for institutions* (supersedes previous version*)
- Fix enrollment status on Special:Student*
- Only add students to Special:Students*
- Accurately report institution activity* (requires a small change to the DB schema*)
- Update summary data when undeleting courses*
- Prevent the undeletion of courses of still-deleted institutions*
- Refactor logic for some redundant data*
- One-time maintenance script to repair summary data*
- Prevent the deletion of institutions that have courses
*Merged
Please note that the rest of this page is about the codebase prior to these patches.
Notes
[edit]- Deleting institutions is dangerous. Really, it should not be possible to delete an institution without first deleting all the related courses. Related: as previously noted, you can undelete a course without undeleting the institution (fixed in patch 6).
- When undeleting institutions, the code attempts to restore the courses that were deleted along with the institution, but a bug (
Org::undelete
, line 124, “Courses” should be “EPCourses”) makes this not work. If we make it impossible to delete an institution that has courses, this won't be a problem, and the actions marked in the table below as missing for undeleting courses would not be required, either. - Under certain circumstances, the
ep_revisions
table can include summary fields. This is incorrect, at least in some cases, though it's unclear if it causes problems. - The
ep_articles
table isn't updated when courses are deleted and undeleted, though it might be. It's unclear whether this causes problems. - I've confirmed that the current code produces incorrect data in the
active_enroll
field of theep_students
table. To remain accurate, the field would have to be updated daily and at numerous transactions (see below). It appears that this field is not used beyond Special:Student. (Instead of trying to keep the summary field up-to-date, patch 2 fixes Special:Student by querying the database for enrollment status on each page view.) - The
active
field in theorgs
table would also have to be updated daily to remain accurate. (Patch 4 provides accurate info in the UI using a different approach.) - It's not clear what should happen to date-related fields in the
ep_students
table when a course's dates are modified. Currently they are left unchanged. Perhaps that's good enough, at least for cases when keeping them consistent would be complicated. - There are certainly issues with modifying institution names, course names and course terms. I haven't investigated them yet, though apparently they wouldn't be database consistency issues.
- The method
ORMRow::loadSummaryFields
is used/overridden in an inconsistent manner. For example: seeCourse::loadSummaryFields
, and note that course summary fields are actually updated inCourse::save
. Compare toOrg::loadSummaryFields
. - Errors in the number of courses and students reported for institutions (see, for example for Red Deer College) are apparently caused by race conditions. (Tentatively fixed in patch 1.)
- It seems that the summary fields with instructor and volunteer stats for institutions (
instructor_count
,oa_count
andca_count
in theep_orgs
table) are never actually read. So long as that remains the case, it isn't necessary to update summary fields in theep_orgs
table when instructors or volunteers are added or removed.
tl;dr A variety of medium-sized bugs. Corruption mainly in summary (redundant) fields. Almost no corruption in canonical data, the only two cases being the undeletion of courses at deleted institutions, and the addition of instructors and volunteers to the ep_students
table.
Analysis by Transaction
[edit]This analysis is for version 3c9339d178018d96e2d0b9d22db83eeece9157f8
of the extension:
Operation | Entry point | Required action | Table | Comments | Happens? | Actual/suggested class and method | Line |
Create course | EditAction::handleSubmission | Insert row | ep_courses | Yes | ORMRow::insert | 408 | |
Initialize summary fields | ep_courses | Yes | Course::save | 237 | |||
Update summary fields | ep_orgs | All are updated; code is confusing | Yes | Course::insert | 126 | ||
Store revision | ep_revisions | Yes | RevisionedObject::insert | 220 | |||
Edit course | EditAction::handleSubmission | Modify row | ep_courses | Summary fields in ep_courses are updated, though it's unnecessary | Yes | ORMRow::saveExisting | 369 |
Update summary fields | ep_orgs | Yes | Course::onUpdated | 201 | |||
Store revision | ep_revisions | Yes | RevisionedObject::onUpdated | 209 | |||
Update active_enroll | ep_students | No | Course::onUpdated | ||||
Delete course | DeleteAction::doDelete | Remove row | ep_courses | Yes | ORMTable::delete | 495 | |
Update summary fields | ep_orgs | Yes | Course::onRemoved | 137 | |||
Delete rows with this course id | ep_users_per_course | Yes | Course::onRemoved | 140 | |||
Store revision | ep_revisions | Yes | RevisionedObject::onRemoved | 234 | |||
Update active_enroll | ep_students | No | Course::onRemoved | ||||
Undelete course | UndeleteAction::doUndelete | Re-insert row | ep_courses | Previous id is also restored; no need to re-save the new revision. | Yes | ORMRow::insert | 408 |
Update summary fields | ep_orgs | No | Course::onUndelete | ||||
Re-create rows with this course id | ep_users_per_course | No | Course::onUndelete | ||||
Update active_enroll | ep_students | No | Course::onUndelete | ||||
Restore course | RestoreAction::doRestore | Restore fields | ep_courses | Yes | ORMRow::saveExisting | 369 | |
Update summary fields | ep_courses | Yes | Course::save | 237 | |||
Update rows with this course id | ep_users_per_course | Yes | Course::onUpdated | 163 | |||
Update summary fields | ep_orgs | Yes | Course::onUpdated | 201 | |||
Store revision | ep_revisions | Yes | RevisionedObject::onUpdated | 209 | |||
Update active_enroll | ep_students | No | Course::onUpdated | ||||
Undo course | UndoAction::doUndo | Modify row | ep_courses | Summary fields in ep_courses are updated, though it's unnecessary | Yes | ORMRow::saveExisting | 369 |
Update summary fields | ep_orgs | Yes | Course::onUpdated | 201 | |||
Store revision | ep_revisions | Yes | RevisionedObject::onUpdated | 209 | |||
Update active_enroll | ep_students | No | Course::onUpdated | ||||
Create org | EditAction::handleSubmission | Insert row | ep_orgs | Yes | ORMRow::insert | 408 | |
Initialize summary fields | ep_orgs | These fields just take DB defaults | Yes | ||||
Store revision | ep_revisions | Yes | RevisionedObject::insert | 220 | |||
Edit org | EditAction::handleSubmission | Modify row | ep_orgs | Yes | ORMRow::saveExisting | 369 | |
Store revision | ep_revisions | Yes | RevisionedObject::onUpdated | 209 | |||
Delete org | DeleteAction::doDelete | Remove row | ep_orgs | Yes | ORMTable::delete | 495 | |
Remove related courses | ep_courses | Yes | ORMTable::delete | 495 | |||
Delete rows with deleted course ids | ep_users_per_course | Yes | Course::onRemoved | 140 | |||
Store revision (org and courses) | ep_revisions | Yes | RevisionedObject::onRemoved | 234 | |||
Update active_enroll | ep_students | No | Course::onRemoved | ||||
Undelete org | UndeleteAction::doUndelete | Re-insert row | ep_orgs | Previous id is also restored; no need to re-save the new revision. | Yes | ORMRow::insert | 408 |
Update summary fields | ep_orgs | Would be needed if courses were restored (see notes): active field might need updating | No | Org::onUndelete | |||
Re-create rows with related course ids | ep_users_per_course | Would be needed if courses were restored (see notes) | No | Course::onUndelete | |||
Update active_enroll | ep_students | Would be needed if courses were restored (see notes) | No | Course::onUndelete | |||
Restore org | RestoreAction::doRestore | Restore fields | ep_orgs | Tries to do something for courses in case name changes, not sure if it works | Yes | ORMRow::saveExisting | 369 |
Store revision | ep_revisions | Yes | RevisionedObject::onUpdated | 209 | |||
Undo org | UndoAction::doUndo | Modify row | ep_orgs | Tries to do something in case name changes. Apparently, summary fields may be revertible (?). | Yes | ORMRow::saveExisting | 369 |
Store revision | ep_revisions | Yes | RevisionedObject::onUpdated | 209 | |||
Enroll student | SpecialEnroll::doEnroll | Update students field | ep_courses | Yes | Course::enlistUsers | 601 | |
Update summary fields | ep_courses | Yes | Course::save | 237 | |||
Add row | ep_users_per_course | Yes | Course::onUpdated | 163 | |||
Update summary fields | ep_orgs | Yes | Course::onUpdated | 201 | |||
Update summary fields | ep_students | Yes | RoleObject::onEnrolled | 314 | |||
Possibly add row | ep_students | Yes | ORMTable::insertRow | 1010 | |||
Remove student | RemoveStudentAction::onView or SpecialDisenroll::doDisenroll | Update students field | ep_courses | Yes | Course::unenlistUsers | 671 | |
Delete article associations | ep_articles | Yes | ArticleStore::deleteArticleByCourseAndUsers | 297 | |||
Update summary fields | ep_courses | Yes | Course::save | 237 | |||
Remove row | ep_users_per_course | Yes | Course::onUpdated | 182 | |||
Update summary fields | ep_orgs | Yes | Course::onUpdated | 201 | |||
Update active_enroll | ep_students | No | Course::onUpdated | ||||
Add instructor/volunteer | ApiEnlist::execute | Update role-related field | ep_courses | Yes | Course::enlistUsers | 601 | |
Update summary fields | ep_courses | Yes | Course::save | 237 | |||
Add row | ep_users_per_course | Yes | Course::onUpdated | 163 | |||
Update summary fields | ep_orgs | Yes | Course::onUpdated | 201 | |||
Possibly add row | ep_students | Rows are incorrectly added to the ep_students table, regardles of the actual role. | Yes, but wrong (fixed in patch 3) | ORMTable::insertRow | 1010 | ||
Remove instructor/volunteer | ApiEnlist::execute | Update role-related field | ep_courses | Yes | Course::unenlistUsers | 671 | |
Update summary fields | ep_courses | Yes | Course::save | 237 | |||
Remove row | ep_users_per_course | Yes | Course::onUpdated | 182 | |||
Assign article | AddArticleAction::onView | Add row | ep_articles | Yes | ArticleStore::insertArticle | 124 | |
Unassign article | RemoveArticleAction::onView | Remove row | ep_articles | Yes | ArticleStore::deleteArticle | 245 | |
Add reviewer | AddReviewerAction::onView | Update reviewers field | ep_articles | Yes | EPArticle::addReviewers | 204 | |
Remove reviewer | RemoveReviewerAction::onView | Update reviewers field | ep_articles | Yes | EPArticle::removeReviewers | 225 |