r109872 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r109871‎ | r109872 | r109873 >
Date:23:35, 23 January 2012
Author:jeroendedauw
Status:deferred
Tags:
Comment:
some initial work on revision support
Modified paths:
  • /trunk/extensions/EducationProgram/EducationProgram.hooks.php (modified) (history)
  • /trunk/extensions/EducationProgram/EducationProgram.i18n.alias.php (modified) (history)
  • /trunk/extensions/EducationProgram/EducationProgram.i18n.php (modified) (history)
  • /trunk/extensions/EducationProgram/EducationProgram.php (modified) (history)
  • /trunk/extensions/EducationProgram/includes/EPDBObject.php (modified) (history)
  • /trunk/extensions/EducationProgram/includes/EPRevision.php (added) (history)
  • /trunk/extensions/EducationProgram/includes/EPRevisions.php (added) (history)
  • /trunk/extensions/EducationProgram/resources/ep.instructor.js (modified) (history)
  • /trunk/extensions/EducationProgram/specials/SpecialCourseHistory.php (added) (history)
  • /trunk/extensions/EducationProgram/specials/SpecialEPFormPage.php (modified) (history)
  • /trunk/extensions/EducationProgram/specials/SpecialEPHistory.php (added) (history)
  • /trunk/extensions/EducationProgram/sql/AddRevisionObjectId.sql (added) (history)
  • /trunk/extensions/EducationProgram/sql/EducationProgram.sql (modified) (history)

Diff [purge]

Index: trunk/extensions/EducationProgram/EducationProgram.i18n.alias.php
——@@ -34,6 +34,7 @@
3535 'OnlineAmbassadors' => array( 'OnlineAmbassadors' ),
3636 'CampusAmbassador' => array( 'CampusAmbassador' ),
3737 'OnlineAmbassador' => array( 'OnlineAmbassador' ),
 38+ 'CourseHistory' => array( 'CourseHistory' ),
3839 );
3940
4041 /** Dutch (Nederlands) */
Index: trunk/extensions/EducationProgram/sql/EducationProgram.sql
——@@ -163,9 +163,10 @@
164164
165165 -- Revision table, holding blobs of various types of objects, such as orgs or students.
166166 -- This is somewhat based on the (core) revision table and is meant to serve
 167+-- as a prototype for a more general system to store this kind of data in a visioned fashion.
167168 CREATE TABLE IF NOT EXISTS /*_*/ep_revisions (
168169 rev_id INT unsigned NOT NULL auto_increment PRIMARY KEY,
 170+ rev_object_id INT unsigned NOT NULL,
169171 rev_type varbinary(32) NOT NULL,
170172 rev_comment TINYBLOB NOT NULL,
171173 rev_user_id INT unsigned NOT NULL default 0,
——@@ -176,6 +177,7 @@
177178 rev_data BLOB NOT NULL
178179 ) /*$wgDBTableOptions*/;
179180
 181+CREATE INDEX /*i*/ep_revision_object_id ON /*_*/ep_revisions (rev_object_id);
180182 CREATE INDEX /*i*/ep_revision_type ON /*_*/ep_revisions (rev_type);
181183 CREATE INDEX /*i*/ep_revision_user_id ON /*_*/ep_revisions (rev_user_id);
182184 CREATE INDEX /*i*/ep_revision_user_text ON /*_*/ep_revisions (rev_user_text);
Index: trunk/extensions/EducationProgram/sql/AddRevisionObjectId.sql
——@@ -0,0 +1,7 @@
 2+-- SQL for the Education Program extension.
 3+-- Add object id field to revision table.
 4+-- Licence: GNU GPL v3+
 5+-- Author: Jeroen De Dauw < jeroendedauw@gmail.com >
 6+
 7+ALTER TABLE /*_*/ep_revisions ADD COLUMN rev_object_id INT unsigned NOT NULL;
 8+CREATE INDEX /*i*/ep_revisions_object_id ON /*_*/ep_revisions (rev_object_id);
Index: trunk/extensions/EducationProgram/specials/SpecialCourseHistory.php
——@@ -0,0 +1,47 @@
 2+<?php
 3+
 4+/**
 5+ * Special page for listing the history of a course.
 6+ *
 7+ * @since 0.1
 8+ *
 9+ * @file SpecialCourseHistory.php
 10+ * @ingroup EducationProgram
 11+ *
 12+ * @licence GNU GPL v3 or later
 13+ * @author Jeroen De Dauw < jeroendedauw@gmail.com >
 14+ */
 15+class SpecialCourseHistory extends SpecialEPHistory {
 16+
 17+ /**
 18+ * Constructor.
 19+ *
 20+ * @since 0.1
 21+ */
 22+ public function __construct() {
 23+ parent::__construct( 'CourseHistory', '', false );
 24+ }
 25+
 26+ /**
 27+ * Main method.
 28+ *
 29+ * @since 0.1
 30+ *
 31+ * @param string $subPage
 32+ */
 33+ public function execute( $subPage ) {
 34+ parent::execute( $subPage );
 35+
 36+ $course = EPCourse::selectRow( null, array( 'id' => $subPage ) );
 37+
 38+ if ( $course === false ) {
 39+ // TODO
 40+ }
 41+ else {
 42+ $this->displayRevisions( $course );
 43+ }
 44+ }
 45+
 46+
 47+
 48+}
Index: trunk/extensions/EducationProgram/specials/SpecialEPFormPage.php
——@@ -1,7 +1,7 @@
22 <?php
33
44 /**
5 - * Extends FormSpecialPage with commons functions needed in EducationProgram.
 5+ * FormSpecialPage equivalent but deriving from SpecialEPPage.
66 *
77 * @since 0.1
88 *
Index: trunk/extensions/EducationProgram/specials/SpecialEPHistory.php
——@@ -0,0 +1,51 @@
 2+<?php
 3+
 4+/**
 5+ * Base class for history special pages.
 6+ *
 7+ * @since 0.1
 8+ *
 9+ * @file SpecialEPHistory.php
 10+ * @ingroup EducationProgram
 11+ *
 12+ * @licence GNU GPL v3 or later
 13+ * @author Jeroen De Dauw < jeroendedauw@gmail.com >
 14+ */
 15+abstract class SpecialEPHistory extends SpecialEPPage {
 16+
 17+ protected function displayRevisions( EPDBObject $object ) {
 18+ $conditions = array(
 19+ 'type' => get_class( $object ),
 20+ );
 21+
 22+ if ( $object->hasIdField() ) {
 23+ $conditions['object_id'] = $object->getId();
 24+ }
 25+
 26+ $revisions = EPRevision::select(
 27+ null,
 28+ $conditions
 29+ );
 30+
 31+ if ( count( $revisions ) > 0 ) {
 32+ array_unshift( $revisions, EPRevision::newFromObject( $object ) );
 33+ $this->displayRevisionList( $revisions );
 34+ }
 35+ else {
 36+ // TODO
 37+ }
 38+ }
 39+
 40+ protected function displayRevisionList( array /* of EPRevision */ $revisions ) {
 41+ foreach ( $revisions as &$revision ) {
 42+ $revision = '<li>' . $this->getRevisionItem( $revision ) . '</li>';
 43+ }
 44+
 45+ $this->getOutput()->addHTML( '<ul>' . implode( '', $revisions ) . '</ul>' );
 46+ }
 47+
 48+ protected function getRevisionItem( EPRevision $revision ) {
 49+ return $revision->getField( 'time' ) . json_encode( $revision->getField( 'data' ) ); // TODO
 50+ }
 51+
 52+}
Index: trunk/extensions/EducationProgram/includes/EPDBObject.php
——@@ -60,6 +60,14 @@
6161 protected $log = true;
6262
6363 /**
 64+ * If the object should store old revisions.
 65+ *
 66+ * @since 0.1
 67+ * @var bool
 68+ */
 69+ protected $storeRevisions = true;
 70+
 71+ /**
6472 * The database connection to use for read operations.
6573 * Can be changed via @see setReadDb.
6674 *
——@@ -320,7 +328,9 @@
321329
322330 switch ( $type ) {
323331 case 'array':
324 - $value = serialize( (array)$value );
 332+ $value = (array)$value;
 333+ case 'blob':
 334+ $value = serialize( $value );
325335 }
326336
327337 $values[$this->getFieldPrefix() . $name] = $value;
——@@ -393,13 +403,14 @@
394404
395405 /**
396406 * Updates the object in the database.
397 - * TODO: store old rev
398407 *
399408 * @since 0.1
400409 *
401410 * @return boolean Success indicator
402411 */
403412 protected function updateInDB() {
 413+ $this->storeRevision();
 414+
404415 $dbw = wfGetDB( DB_MASTER );
405416
406417 $success = $dbw->update(
——@@ -416,6 +427,16 @@
417428 return $success;
418429 }
419430
 431+ protected function storeRevision( $isDelete = false ) {
 432+ static::setReadDb( DB_MASTER );
 433+ $revison = static::selectRow( null, array( 'id' => $this->getId() ) );
 434+ static::setReadDb( DB_SLAVE );
 435+
 436+ $revison = EPRevision::newFromObject( $revison, $isDelete );
 437+
 438+ $revison->writeToDB();
 439+ }
 440+
420441 /**
421442 * Inserts the object into the database.
422443 *
——@@ -443,13 +464,14 @@
444465
445466 /**
446467 * Removes the object from the database.
447 - * TODO: store rev
448468 *
449469 * @since 0.1
450470 *
451471 * @return boolean Success indicator
452472 */
453473 public function removeFromDB() {
 474+ $this->storeRevision( true );
 475+
454476 $success = $this->delete( array( 'id' => $this->getId() ) );
455477
456478 if ( $success ) {
——@@ -552,6 +574,11 @@
553575 $value = array();
554576 }
555577 break;
 578+ case 'blob':
 579+ if ( is_string( $value ) ) {
 580+ $value = unserialize( $value );
 581+ }
 582+ break;
556583 case 'id':
557584 if ( is_string( $value ) ) {
558585 $value = (int)$value;
——@@ -1132,7 +1159,8 @@
11331160 'float' => 'NULL',
11341161 'str' => 'string',
11351162 'bool' => 'integer',
1136 - 'array' => 'string'
 1163+ 'array' => 'string',
 1164+ 'blob' => 'string',
11371165 );
11381166
11391167 $params = array();
——@@ -1207,6 +1235,17 @@
12081236 }
12091237
12101238 /**
 1239+ * Sets the value for the @see $storeRevisions field.
 1240+ *
 1241+ * @since 0.1
 1242+ *
 1243+ * @param boolean $store
 1244+ */
 1245+ public function setStoreRevisions( $store ) {
 1246+ $this->storeRevisions = $store;
 1247+ }
 1248+
 1249+ /**
12111250 * Sets the value for the @see $log field.
12121251 *
12131252 * @since 0.1
Index: trunk/extensions/EducationProgram/includes/EPRevisions.php
——@@ -0,0 +1,24 @@
 2+<?php
 3+
 4+/**
 5+ * Static class for storing and retrieving revisions of EPDBObjects.
 6+ *
 7+ * @since 0.1
 8+ *
 9+ * @file EPRevisions.php
 10+ * @ingroup EducationProgram
 11+ *
 12+ * @licence GNU GPL v3 or later
 13+ * @author Jeroen De Dauw < jeroendedauw@gmail.com >
 14+ */
 15+final class EPRevisions {
 16+
 17+ public static function storeRevision() {
 18+
 19+ }
 20+
 21+ public static function getRevision() {
 22+
 23+ }
 24+
 25+}
Index: trunk/extensions/EducationProgram/includes/EPRevision.php
——@@ -0,0 +1,80 @@
 2+<?php
 3+
 4+/**
 5+ * Class representing a single revision.
 6+ *
 7+ * @since 0.1
 8+ *
 9+ * @file EPRevision.php
 10+ * @ingroup EducationProgram
 11+ *
 12+ * @licence GNU GPL v3 or later
 13+ * @author Jeroen De Dauw < jeroendedauw@gmail.com >
 14+ */
 15+class EPRevision extends EPDBObject {
 16+
 17+ /**
 18+ * @see parent::__construct
 19+ *
 20+ * @since 0.1
 21+ *
 22+ * @param array|null $fields
 23+ * @param bool $loadDefaults
 24+ */
 25+ public function __construct( $fields = null, $loadDefaults = false ) {
 26+ $this->setStoreRevisions( false );
 27+ parent::__construct( $fields, $loadDefaults );
 28+ }
 29+
 30+ /**
 31+ * @see parent::getFieldTypes
 32+ *
 33+ * @since 0.1
 34+ *
 35+ * @return array
 36+ */
 37+ protected static function getFieldTypes() {
 38+ return array(
 39+ 'id' => 'id',
 40+
 41+ 'object_id' => 'id',
 42+ 'user_id' => 'id',
 43+ 'type' => 'str',
 44+ 'comment' => 'str',
 45+ 'user_text' => 'str',
 46+ 'minor_edit' => 'bool',
 47+ 'time' => 'str', // TS_MW
 48+ 'deleted' => 'bool',
 49+ 'data' => 'blob',
 50+ );
 51+ }
 52+
 53+ /**
 54+ * Create a new revision object for the provided EPDBObject.
 55+ * The EPDBObject should have all it's fields loaded.
 56+ *
 57+ * @since 0.1
 58+ *
 59+ * @param EPDBObject $object
 60+ * @param boolean $deleted
 61+ *
 62+ * @return EPRevision
 63+ */
 64+ public static function newFromObject( EPDBObject $object, $deleted = false ) {
 65+ $fields = array(
 66+ 'object_id' => $object->getId(),
 67+ 'user_id' => $GLOBALS['wgUser']->getID(), // TODO
 68+ 'user_text' => $GLOBALS['wgUser']->getName(), // TODO
 69+ 'type' => get_class( $object ),
 70+ 'comment' => '', // TODO
 71+ 'minor_edit' => false, // TODO
 72+ 'time' => wfTimestampNow(),
 73+ 'deleted' => $deleted,
 74+ 'data' => serialize( $object->toArray() )
 75+ );
 76+
 77+ return new static( $fields );
 78+ }
 79+
 80+
 81+}
Index: trunk/extensions/EducationProgram/EducationProgram.i18n.php
——@@ -25,9 +25,9 @@
2626 'ep-toplink' => 'My courses',
2727
2828 // Tabs
29 - 'ep-tab-view' => 'View',
 29+ 'ep-tab-view' => 'Read',
3030 'ep-tab-edit' => 'Edit',
31 - 'ep-tab-history' => 'history',
 31+ 'ep-tab-history' => 'View history',
3232 'ep-tab-enroll' => 'Enroll',
3333
3434 // Tooltips
——@@ -405,6 +405,7 @@
406406 'ep-mycourses-not-a-student' => 'You are not enrolled in any [[Special:Courses|courses]].',
407407
408408 // ep.instructor
 409+ // <script>alert("XSS");</script>
409410 'ep-instructor-remove-title' => 'Remove instructor from master course',
410411 'ep-instructor-remove-button' => 'Remove instructor',
411412 'ep-instructor-removing' => 'Removing...',
Index: trunk/extensions/EducationProgram/EducationProgram.php
——@@ -81,6 +81,7 @@
8282 $wgAutoloadClasses['EPCA'] = dirname( __FILE__ ) . '/includes/EPCA.php';
8383 $wgAutoloadClasses['EPCAPager'] = dirname( __FILE__ ) . '/includes/EPCAPager.php';
8484 $wgAutoloadClasses['EPHTMLDateField'] = dirname( __FILE__ ) . '/includes/EPHTMLDateField.php';
 85+$wgAutoloadClasses['EPRevision'] = dirname( __FILE__ ) . '/includes/EPRevision.php';
8586
8687 $wgAutoloadClasses['SpecialCourse'] = dirname( __FILE__ ) . '/specials/SpecialCourse.php';
8788 $wgAutoloadClasses['SpecialCourses'] = dirname( __FILE__ ) . '/specials/SpecialCourses.php';
——@@ -102,6 +103,8 @@
103104 $wgAutoloadClasses['SpecialOAs'] = dirname( __FILE__ ) . '/specials/SpecialOAs.php';
104105 $wgAutoloadClasses['SpecialCA'] = dirname( __FILE__ ) . '/specials/SpecialCA.php';
105106 $wgAutoloadClasses['SpecialOA'] = dirname( __FILE__ ) . '/specials/SpecialOA.php';
 107+$wgAutoloadClasses['SpecialEPHistory'] = dirname( __FILE__ ) . '/specials/SpecialEPHistory.php';
 108+$wgAutoloadClasses['SpecialCourseHistory'] = dirname( __FILE__ ) . '/specials/SpecialCourseHistory.php';
106109
107110 // Special pages
108111 $wgSpecialPages['MyCourses'] = 'SpecialMyCourses';
——@@ -122,6 +125,7 @@
123126 $wgSpecialPages['OnlineAmbassadors'] = 'SpecialOAs';
124127 $wgSpecialPages['CampusAmbassador'] = 'SpecialCA';
125128 $wgSpecialPages['OnlineAmbassador'] = 'SpecialOA';
 129+$wgSpecialPages['CourseHistory'] = 'SpecialCourseHistory';
126130
127131 $wgSpecialPageGroups['MyCourses'] = 'education';
128132 $wgSpecialPageGroups['Institution'] = 'education';
——@@ -140,9 +144,11 @@
141145 $wgSpecialPageGroups['OnlineAmbassadors'] = 'education';
142146 $wgSpecialPageGroups['CampusAmbassador'] = 'education';
143147 $wgSpecialPageGroups['OnlineAmbassador'] = 'education';
 148+$wgSpecialPageGroups['CourseHistory'] = 'education';
144149
145150 // DB object classes
146151 $egEPDBObjects = array();
 152+$egEPDBObjects['EPRevision'] = array( 'table' => 'ep_revisions', 'prefix' => 'rev_' );
147153 $egEPDBObjects['EPOrg'] = array( 'table' => 'ep_orgs', 'prefix' => 'org_' );
148154 $egEPDBObjects['EPMC'] = array( 'table' => 'ep_mcs', 'prefix' => 'mc_' );
149155 $egEPDBObjects['EPCourse'] = array( 'table' => 'ep_courses', 'prefix' => 'course_' );
Index: trunk/extensions/EducationProgram/resources/ep.instructor.js
——@@ -83,7 +83,9 @@
8484 '<b>' + mw.html.escape( bestName ) + '</b>',
8585 '<b>' + mw.html.escape( mcName ) + '</b>'
8686 ) );
87 -
 87+
 88+ //$dialog.append( $( '<p>' ).msg( 'ep-instructor-remove-title' ) );
 89+
8890 $dialog.append( summaryInput );
8991
9092 summaryInput.focus();
Index: trunk/extensions/EducationProgram/EducationProgram.hooks.php
——@@ -37,6 +37,14 @@
3838 true
3939 ) );
4040
 41+ $updater->addExtensionUpdate( array(
 42+ 'addField',
 43+ 'ep_revisions',
 44+ 'rev_object_id',
 45+ dirname( __FILE__ ) . '/sql/AddRevisionObjectId.sql',
 46+ true
 47+ ) );
 48+
4149 return true;
4250 }
4351
——@@ -179,7 +187,7 @@
180188 array(
181189 'view' => 'Course',
182190 'edit' => 'EditCourse',
183 - //'history' => 'CourseHistory',
 191+ 'history' => 'CourseHistory',
184192 'enroll' => 'Enroll',
185193 ),
186194 );
——@@ -237,8 +245,7 @@
238246 $viewLinks['history'] = array(
239247 'class' => $type === 'history' ? 'selected' : false,
240248 'text' => wfMsg( 'ep-tab-history' ),
241 - 'href' => '' // TODO
242 - //SpecialPage::getTitleFor( $specialSet['history'], $textParts[1] )->getLocalUrl()
 249+ 'href' => SpecialPage::getTitleFor( $specialSet['history'], $textParts[1] )->getLocalUrl()
243250 );
244251
245252 if ( $canonicalSet['view'] === 'Course' ) {

Follow-up revisions

RevisionCommit summaryAuthorDate
r109876Follow up to r109872; added extra special pagesjeroendedauw23:58, 23 January 2012
r109890Follow up to r109872; r109876; make use of chronological pager to list revsjeroendedauw01:16, 24 January 2012
r109939Follow up to r109872; fixed recursion issue and registration of special pagesjeroendedauw18:58, 24 January 2012

Status & tagging log