Jump to content

Wikipedia Education Program/Database Analysis Notes

From mediawiki.org

Patches

[edit]

*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 the ep_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 the orgs 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: see Course::loadSummaryFields, and note that course summary fields are actually updated in Course::save. Compare to Org::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 and ca_count in the ep_orgs table) are never actually read. So long as that remains the case, it isn't necessary to update summary fields in the ep_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