r108221 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r108220‎ | r108221 | r108222 >
Date:12:01, 6 January 2012
Author:jeroendedauw
Status:deferred
Tags:educationprogram 
Comment:
initial work on summary fields
Modified paths:
  • /trunk/extensions/EducationProgram/EducationProgram.i18n.php (modified) (history)
  • /trunk/extensions/EducationProgram/api/ApiDeleteEducation.php (modified) (history)
  • /trunk/extensions/EducationProgram/includes/EPCourse.php (modified) (history)
  • /trunk/extensions/EducationProgram/includes/EPDBObject.php (modified) (history)
  • /trunk/extensions/EducationProgram/includes/EPOrg.php (modified) (history)
  • /trunk/extensions/EducationProgram/includes/EPOrgPager.php (modified) (history)
  • /trunk/extensions/EducationProgram/includes/EPPager.php (modified) (history)
  • /trunk/extensions/EducationProgram/includes/EPStudent.php (modified) (history)
  • /trunk/extensions/EducationProgram/includes/EPTerm.php (modified) (history)
  • /trunk/extensions/EducationProgram/sql/AddExtraFields.sql (modified) (history)
  • /trunk/extensions/EducationProgram/sql/EducationProgram.sql (modified) (history)

Diff [purge]

Index: trunk/extensions/EducationProgram/sql/AddExtraFields.sql
@@ -5,10 +5,12 @@
66
77 ALTER TABLE /*_*/ep_orgs ADD COLUMN org_courses SMALLINT unsigned NOT NULL;
88 ALTER TABLE /*_*/ep_orgs ADD COLUMN org_mentors SMALLINT unsigned NOT NULL;
 9+ALTER TABLE /*_*/ep_orgs ADD COLUMN org_terms SMALLINT unsigned NOT NULL;
910 ALTER TABLE /*_*/ep_orgs ADD COLUMN org_students INT unsigned NOT NULL;
1011
1112 CREATE INDEX /*i*/ep_org_courses ON /*_*/ep_orgs (org_courses);
1213 CREATE INDEX /*i*/ep_org_mentors ON /*_*/ep_orgs (org_mentors);
 14+CREATE INDEX /*i*/ep_org_terms ON /*_*/ep_orgs (org_terms);
1315 CREATE INDEX /*i*/ep_org_students ON /*_*/ep_orgs (org_students);
1416
1517 ALTER TABLE /*_*/ep_courses ADD COLUMN course_lang VARCHAR(10) NOT NULL;
Index: trunk/extensions/EducationProgram/sql/EducationProgram.sql
@@ -11,11 +11,13 @@
1212 org_country VARCHAR(255) NOT NULL, -- Name of the country where the org is located
1313
1414 org_courses SMALLINT unsigned NOT NULL, -- Amount of courses
 15+ org_terms SMALLINT unsigned NOT NULL, -- Amount of terms
1516 org_mentors SMALLINT unsigned NOT NULL, -- Amount of mentors
1617 org_students INT unsigned NOT NULL -- Amount of students
1718 ) /*$wgDBTableOptions*/;
1819
1920 CREATE UNIQUE INDEX /*i*/ep_org_name ON /*_*/ep_orgs (org_name);
 21+CREATE INDEX /*i*/ep_org_terms ON /*_*/ep_orgs (org_terms);
2022 CREATE INDEX /*i*/ep_org_courses ON /*_*/ep_orgs (org_courses);
2123 CREATE INDEX /*i*/ep_org_mentors ON /*_*/ep_orgs (org_mentors);
2224 CREATE INDEX /*i*/ep_org_students ON /*_*/ep_orgs (org_students);
Index: trunk/extensions/EducationProgram/includes/EPOrgPager.php
@@ -32,6 +32,10 @@
3333 'name',
3434 'city',
3535 'country',
 36+ 'courses',
 37+ 'terms',
 38+ 'mentors',
 39+ 'students',
3640 );
3741 }
3842
@@ -65,8 +69,11 @@
6670 break;
6771 case 'country':
6872 $countries = array_flip( efEpGetCountryOptions( $this->getLanguage()->getCode() ) );
69 - $value = $countries[$value];
 73+ $value = htmlspecialchars( $countries[$value] );
7074 break;
 75+ case 'courses': case 'mentors': case 'students': case 'terms':
 76+ $value = htmlspecialchars( $this->getLanguage()->formatNum( $value ) );
 77+ break;
7178 }
7279
7380 return $value;
@@ -81,6 +88,11 @@
8289 'name',
8390 'city',
8491 'country',
 92+ 'courses',
 93+ 'mentors',
 94+ 'students',
 95+ 'terms',
 96+ 'terms',
8597 );
8698 }
8799
Index: trunk/extensions/EducationProgram/includes/EPStudent.php
@@ -32,6 +32,11 @@
3333 return array(
3434 'id' => 'id',
3535 'user_id' => 'id',
 36+
 37+ 'first_enroll' => 'str', // TS_MW
 38+
 39+ 'last_active' => 'str', // TS_MW
 40+ 'active_enroll' => 'bool',
3641 );
3742 }
3843
Index: trunk/extensions/EducationProgram/includes/EPOrg.php
@@ -43,6 +43,11 @@
4444 'name' => 'str',
4545 'city' => 'str',
4646 'country' => 'str',
 47+
 48+ 'courses' => 'int',
 49+ 'terms' => 'int',
 50+ 'mentors' => 'int',
 51+ 'students' => 'int',
4752 );
4853 }
4954
@@ -55,8 +60,62 @@
5661 'name' => '',
5762 'city' => '',
5863 'country' => '',
 64+
 65+ 'courses' => 0,
 66+ 'terms' => 0,
 67+ 'mentors' => 0,
 68+ 'students' => 0,
5969 );
6070 }
 71+
 72+ /**
 73+ * (non-PHPdoc)
 74+ * @see EPDBObject::loadSummaryFields()
 75+ */
 76+ public function loadSummaryFields( $summaryFields = null ) {
 77+ if ( is_null( $summaryFields ) ) {
 78+ $summaryFields = array( 'courses', 'terms', 'mentors', 'students' );
 79+ }
 80+ else {
 81+ $summaryFields = (array)$summaryFields;
 82+ }
 83+
 84+ $fields = array();
 85+
 86+ if ( in_array( 'courses', $summaryFields ) ) {
 87+ $fields['courses'] = EPCourse::count( array( 'org_id' => $this->getId() ) );
 88+ }
 89+
 90+ if ( in_array( 'terms', $summaryFields ) ) {
 91+ $fields['terms'] = EPTerm::count( array( 'org_id' => $this->getId() ) );
 92+ }
 93+
 94+ $dbr = wfGetDB( DB_SLAVE );
 95+
 96+ if ( in_array( 'mentors', $summaryFields ) ) {
 97+ $fields['mentors'] = $dbr->select(
 98+ 'ep_mentors_per_org',
 99+ 'COUNT(*) AS rowcount',
 100+ array( 'mpo_org_id' => $this->getId() )
 101+ );
 102+
 103+ $fields['mentors'] = $fields['mentors']->rowcount;
 104+ }
 105+
 106+ if ( in_array( 'students', $summaryFields ) ) {
 107+ $termIds = EPTerm::selectFields( 'id', array( 'org_id' => $this->getId() ) );
 108+
 109+ $fields['students'] = $dbr->select(
 110+ 'ep_students_per_term',
 111+ 'COUNT(*) AS rowcount',
 112+ array( 'spt_term_id' => $termIds )
 113+ );
 114+
 115+ $fields['students'] = $fields['students']->rowcount;
 116+ }
 117+
 118+ $this->setFields( $fields );
 119+ }
61120
62121 /**
63122 * (non-PHPdoc)
Index: trunk/extensions/EducationProgram/includes/EPTerm.php
@@ -74,6 +74,8 @@
7575 $this->setField( 'org_id', $this->getCourse( 'org_id' )->getField( 'org_id' ) );
7676 }
7777
 78+ EPOrg::updateSummaryFields( 'terms', array( 'id' => $this->getField( 'org_id' ) ) );
 79+
7880 return parent::insertIntoDB();
7981 }
8082
@@ -83,15 +85,38 @@
8486 */
8587 public function removeFromDB() {
8688 $id = $this->getId();
 89+ $this->loadFields( array( 'org_id' ) );
 90+ $orgId = $this->getField( 'org_id', false );
8791
8892 $success = parent::removeFromDB();
8993
9094 if ( $success ) {
9195 $success = wfGetDB( DB_MASTER )->delete( 'ep_students_per_term', array( 'spt_term_id' => $id ) ) && $success;
9296 }
93 -
 97+
 98+ if ( $orgId !== false ) {
 99+ EPOrg::updateSummaryFields( array( 'terms', 'students' ), array( 'id' => $orgId ) );
 100+ }
 101+
94102 return $success;
95103 }
 104+
 105+ /**
 106+ * (non-PHPdoc)
 107+ * @see EPDBObject::updateInDB()
 108+ */
 109+ protected function updateInDB() {
 110+ $oldOrgId = $this->hasField( 'org_id' ) ? self::selectFieldsRow( 'org_id', array( 'id' => $this->getId() ) ) : false;
 111+
 112+ $success = parent::updateInDB();
 113+
 114+ if ( $success && $oldOrgId !== false && $oldOrgId !== $this->getField( 'org_id' ) ) {
 115+ $conds = array( 'id' => array( $oldOrgId, $this->getField( 'org_id' ) ) );
 116+ EPOrg::updateSummaryFields( array( 'terms', 'students' ), $conds );
 117+ }
 118+
 119+ return $success;
 120+ }
96121
97122 /**
98123 * Returns the course associated with this term.
Index: trunk/extensions/EducationProgram/includes/EPDBObject.php
@@ -168,13 +168,16 @@
169169 * @since 0.1
170170 *
171171 * @param string $name
 172+ * @param mixed $default
172173 *
173174 * @throws MWException
174175 * @return mixed
175176 */
176 - public function getField( $name ) {
 177+ public function getField( $name, $default = null ) {
177178 if ( $this->hasField( $name ) ) {
178179 return $this->fields[$name];
 180+ } elseif ( !is_null( $default ) ) {
 181+ return $default;
179182 } else {
180183 throw new MWException( 'Attempted to get not-set field ' . $name );
181184 }
@@ -343,6 +346,7 @@
344347
345348 /**
346349 * Updates the object in the database.
 350+ * TODO: log and store old rev
347351 *
348352 * @since 0.1
349353 *
@@ -361,6 +365,7 @@
362366
363367 /**
364368 * Inserts the object into the database.
 369+ * TODO: log
365370 *
366371 * @since 0.1
367372 *
@@ -383,6 +388,7 @@
384389
385390 /**
386391 * Removes the object from the database.
 392+ * TODO: log and store rev
387393 *
388394 * @since 0.1
389395 *
@@ -1066,4 +1072,34 @@
10671073 return $params;
10681074 }
10691075
 1076+ /**
 1077+ * Computes and updates the values of the summary fields.
 1078+ *
 1079+ * @since 0.1
 1080+ *
 1081+ * @param array|string|null $summaryFields
 1082+ */
 1083+ public function loadSummaryFields( $summaryFields = null ) {
 1084+
 1085+ }
 1086+
 1087+ /**
 1088+ * Computes the values of the summary fields of the objects matching the provided conditions.
 1089+ *
 1090+ * @since 0.1
 1091+ *
 1092+ * @param array|string|null $summaryFields
 1093+ * @param array $conditions
 1094+ */
 1095+ public static function updateSummaryFields( $summaryFields = null, array $conditions = array() ) {
 1096+ self::setReadDb( DB_MASTER );
 1097+
 1098+ foreach ( self::select( 'id', $conditions ) as /* EPDBObject */ $item ) {
 1099+ $item->loadSummaryFields( $summaryFields );
 1100+ $item->writeToDB();
 1101+ }
 1102+
 1103+ self::setReadDb( DB_SLAVE );
 1104+ }
 1105+
10701106 }
Index: trunk/extensions/EducationProgram/includes/EPPager.php
@@ -492,7 +492,8 @@
493493
494494 /**
495495 * Similar to TablePager::formatValue, but passes along the name of the field without prefix.
496 - *
 496+ * Returned values need to be escaped!
 497+ *
497498 * @since 0.1
498499 *
499500 * @param string $name
Index: trunk/extensions/EducationProgram/includes/EPCourse.php
@@ -36,6 +36,9 @@
3737
3838 'name' => 'str',
3939 'description' => 'str',
 40+ 'lang' => 'str',
 41+
 42+ 'students' => 'int',
4043 );
4144 }
4245
Index: trunk/extensions/EducationProgram/EducationProgram.i18n.php
@@ -128,6 +128,10 @@
129129 'eporgpager-header-city' => 'City',
130130 'eporgpager-header-country' => 'Country',
131131 'eporgpager-filter-country' => 'Country',
 132+ 'eporgpager-header-courses' => 'Courses',
 133+ 'eporgpager-header-mentors' => 'Ambassadors',
 134+ 'eporgpager-header-students' => 'Students',
 135+ 'eporgpager-header-terms' => 'Terms',
132136
133137 // Institution pager
134138 'epcoursepager-header-name' => 'Name',
Index: trunk/extensions/EducationProgram/api/ApiDeleteEducation.php
@@ -24,7 +24,7 @@
2525 protected static $typeMap = array(
2626 'org' => 'EPOrg',
2727 'course' => 'EPCourse',
28 - 'trem' => 'EPTerm',
 28+ 'term' => 'EPTerm',
2929 'student' => 'EPStudent',
3030 'mentor' => 'EPMentor',
3131 );

Status & tagging log