r109667 MediaWiki - Code Review archive

Repository:	MediaWiki
Revision:	< r109666‎ \| r109667 \| r109668 >
Date:	00:36, 21 January 2012
Author:	jeroendedauw
Status:	deferred
Tags:
Comment:	follow up to r109656 - renaming and added tables for online and campus ambassadors
Modified paths:	/trunk/extensions/EducationProgram/EducationProgram.php (modified) (history) /trunk/extensions/EducationProgram/includes/EPOrg.php (modified) (history) /trunk/extensions/EducationProgram/sql/EducationProgram.sql (modified) (history)

Diff [purge]

Index: trunk/extensions/EducationProgram/sql/EducationProgram.sql
—	—	@@ -12,7 +12,7 @@
13	13
14	14	org_active TINYINT unsigned NOT NULL, -- If the org has any active terms
15	15	org_courses SMALLINT unsigned NOT NULL, -- Amount of courses
16		~~- org_terms SMALLINT unsigned NOT NULL, -- Amount of terms~~
	16	+ org_mcs SMALLINT unsigned NOT NULL, -- Amount of master courses
17	17	org_students INT unsigned NOT NULL -- Amount of students
18	18	) /$wgDBTableOptions/;
19	19
—	—	@@ -22,6 +22,8 @@
23	23	CREATE INDEX /i/ep_org_students ON /_/ep_orgs (org_students);
24	24	CREATE INDEX /i/ep_org_active ON /_/ep_orgs (org_active);
25	25
	26	+
	27	+
26	28	-- Master courses. These describe a specific course, time-independent.
27	29	CREATE TABLE IF NOT EXISTS /_/ep_mcs (
28	30	mc_id INT unsigned NOT NULL auto_increment PRIMARY KEY,
—	—	@@ -42,6 +44,8 @@
43	45	CREATE INDEX /i/ep_mc_students ON /_/ep_mcs (mc_students);
44	46	CREATE INDEX /i/ep_mc_active ON /_/ep_mcs (mc_active);
45	47
	48	+
	49	+
46	50	-- Courses. These are "instances" of a master course in a certain period.
47	51	CREATE TABLE IF NOT EXISTS /_/ep_courses (
48	52	course_id INT unsigned NOT NULL auto_increment PRIMARY KEY,
—	—	@@ -65,6 +69,8 @@
66	70	CREATE UNIQUE INDEX /i/ep_trem_period ON /_/ep_courses (course_org_id, course_start, course_end);
67	71	CREATE INDEX /i/ep_course_students ON /_/ep_courses (course_students);
68	72
	73	+
	74	+
69	75	-- Students. In essence this is an extension to the user table.
70	76	CREATE TABLE IF NOT EXISTS /_/ep_students (
71	77	student_id INT unsigned NOT NULL auto_increment PRIMARY KEY,
—	—	@@ -81,15 +87,7 @@
82	88	CREATE INDEX /i/ep_students_last_active ON /_/ep_students (student_last_active);
83	89	CREATE INDEX /i/ep_students_active_enroll ON /_/ep_students (student_active_enroll);
84	90
85		~~-CREATE TABLE IF NOT EXISTS /_/ep_mentors (~~
86		~~- mentor_id INT unsigned NOT NULL auto_increment PRIMARY KEY,~~
87		~~- mentor_user_id INT unsigned NOT NULL -- Foreign key on user.user_id~~
88		~~-) /$wgDBTableOptions/;~~
89		-
90		~~-CREATE UNIQUE INDEX /i/ep_mentors_user_id ON /_/ep_mentors (mentor_user_id);~~
91		-
	91	+-- Links the students with their courses.
92	92	CREATE TABLE IF NOT EXISTS /_/ep_students_per_course (
93	93	spc_student_id INT unsigned NOT NULL, -- Foreign key on ep_students.student_id
94	94	spc_course_id INT unsigned NOT NULL -- Foreign key on ep_courses.course_id
—	—	@@ -97,6 +95,26 @@
98	96
99	97	CREATE UNIQUE INDEX /i/ep_students_per_course ON /_/ep_students_per_course (spc_student_id, spc_course_id);
100	98
	99	+
	100	+
	101	+-- Instructors. In essence this is an extension to the user table.
	102	+CREATE TABLE IF NOT EXISTS /_/ep_instructors (
	103	+ instructor_id INT unsigned NOT NULL auto_increment PRIMARY KEY,
	104	+ instructor_user_id INT unsigned NOT NULL -- Foreign key on user.user_id
	105	+) /$wgDBTableOptions/;
	106	+
	107	+CREATE UNIQUE INDEX /i/ep_instructors_user_id ON /_/ep_instructors (instructor_user_id);
	108	+
	109	+
	110	+
	111	+-- Campus ambassadors. In essence this is an extension to the user table.
	112	+CREATE TABLE IF NOT EXISTS /_/ep_cas (
	113	+ ca_id INT unsigned NOT NULL auto_increment PRIMARY KEY,
	114	+ ca_user_id INT unsigned NOT NULL -- Foreign key on user.user_id
	115	+) /$wgDBTableOptions/;
	116	+
	117	+CREATE UNIQUE INDEX /i/ep_cas_user_id ON /_/ep_cas (ca_user_id);
	118	+
101	119	-- Links the campus ambassadors with all their orgs.
102	120	CREATE TABLE IF NOT EXISTS /_/ep_cas_per_org (
103	121	cpo_ca_id INT unsigned NOT NULL, -- Foreign key on ep_cas.ca_id
—	—	@@ -105,6 +123,18 @@
106	124
107	125	CREATE UNIQUE INDEX /i/ep_cas_per_org ON /_/ep_cas_per_org (cpo_ca_id, cpo_org_id);
108	126
	127	+
	128	+
	129	+-- Online ambassadors. In essence this is an extension to the user table.
	130	+CREATE TABLE IF NOT EXISTS /_/ep_oas (
	131	+ oa_id INT unsigned NOT NULL auto_increment PRIMARY KEY,
	132	+ oa_user_id INT unsigned NOT NULL -- Foreign key on user.user_id
	133	+) /$wgDBTableOptions/;
	134	+
	135	+CREATE UNIQUE INDEX /i/ep_oas_user_id ON /_/ep_oas (oa_user_id);
	136	+
	137	+
	138	+
109	139	-- Revision table, holding blobs of various types of objects, such as orgs or students.
110	140	-- This is somewhat based on the (core) revision table and is meant to serve
111	141	-- as a prototype for a more general system to store this kind of data in a versioned fashion.
Index: trunk/extensions/EducationProgram/includes/EPOrg.php
—	—	@@ -14,20 +14,20 @@
15	15	class EPOrg extends EPDBObject {
16	16
17	17	/**
18		~~- * Cached array of the linked EPCourse objects.~~
	18	+ * Cached array of the linked EPMC objects.
19	19	*
20	20	* @since 0.1
21	21	* @var array\|false
22	22	*/
23		~~- protected $courses = false;~~
	23	+ protected $mcs = false;
24	24
25	25	/**
26		~~- * Cached array of the linked EPTerm objects.~~
	26	+ * Cached array of the linked EPCourse objects.
27	27	*
28	28	* @since 0.1
29	29	* @var array\|false
30	30	*/
31		~~- protected $terms = false;~~
	31	+ protected $courses = false;
32	32
33	33	/**
34	34	* @see parent::getFieldTypes
—	—	@@ -46,7 +46,7 @@
47	47
48	48	'active' => 'bool',
49	49	'courses' => 'int',
50		~~- 'terms' => 'int',~~
	50	+ 'mcs' => 'int',
51	51	'students' => 'int',
52	52	);
53	53	}
—	—	@@ -63,7 +63,7 @@
64	64
65	65	'active' => false,
66	66	'courses' => 0,
67		~~- 'terms' => 0,~~
	67	+ 'mcs' => 0,
68	68	'students' => 0,
69	69	);
70	70	}
—	—	@@ -85,7 +85,7 @@
86	86	*/
87	87	public function loadSummaryFields( $summaryFields = null ) {
88	88	if ( is_null( $summaryFields ) ) {
89		~~- $summaryFields = array( 'courses', 'terms', 'students', 'active' );~~
	89	+ $summaryFields = array( 'courses', 'mcs', 'students', 'active' );
90	90	}
91	91	else {
92	92	$summaryFields = (array)$summaryFields;
—	—	@@ -97,20 +97,20 @@
98	98	$fields['courses'] = EPCourse::count( array( 'org_id' => $this->getId() ) );
99	99	}
100	100
101		~~- if ( in_array( 'terms', $summaryFields ) ) {~~
102		~~- $fields['terms'] = EPTerm::count( array( 'org_id' => $this->getId() ) );~~
	101	+ if ( in_array( 'mcs', $summaryFields ) ) {
	102	+ $fields['mcs'] = EPMC::count( array( 'org_id' => $this->getId() ) );
103	103	}
104	104
105	105	$dbr = wfGetDB( DB_SLAVE );
106	106
107	107	if ( in_array( 'students', $summaryFields ) ) {
108		~~- $termIds = EPTerm::selectFields( 'id', array( 'org_id' => $this->getId() ) );~~
	108	+ $courseIds = EPCourse::selectFields( 'id', array( 'org_id' => $this->getId() ) );
109	109
110		~~- if ( count( $termIds ) > 0 ) {~~
	110	+ if ( count( $courseIds ) > 0 ) {
111	111	$fields['students'] = $dbr->select(
112		~~- 'ep_students_per_term',~~
	112	+ 'ep_students_per_course',
113	113	'COUNT(*) AS rowcount',
114		~~- array( 'spt_term_id' => $termIds )~~
	114	+ array( 'spc_course_id' => $courseIds )
115	115	);
116	116
117	117	$fields['students'] = $fields['students']->fetchObject()->rowcount;
—	—	@@ -123,7 +123,7 @@
124	124	if ( in_array( 'active', $summaryFields ) ) {
125	125	$now = wfGetDB( DB_SLAVE )->addQuotes( wfTimestampNow() );
126	126
127		~~- $fields['active'] = EPTerm::has( array(~~
	127	+ $fields['active'] = EPCourse::has( array(
128	128	'org_id' => $this->getId(),
129	129	'end >= ' . $now,
130	130	'start <= ' . $now,
—	—	@@ -144,10 +144,10 @@
145	145	$success = parent::removeFromDB();
146	146
147	147	if ( $success ) {
148		~~- $success = wfGetDB( DB_MASTER )->delete( 'ep_mentors_per_org', array( 'mpo_org_id' => $id ) ) && $success;~~
	148	+ $success = wfGetDB( DB_MASTER )->delete( 'ep_cas_per_org', array( 'cpo_org_id' => $id ) ) && $success;
149	149
150		~~- foreach ( EPCourse::select( 'id', array( 'org_id' => $id ) ) as /* EPCourse */ $course ) {~~
151		~~- $success = $course->removeFromDB() && $success;~~
	150	+ foreach ( EPMC::select( 'id', array( 'org_id' => $id ) ) as /* EPMC */ $masterCourse ) {
	151	+ $success = $masterCourse->removeFromDB() && $success;
152	152	}
153	153	}
154	154
—	—	@@ -257,37 +257,37 @@
258	258	}
259	259
260	260	/**
261		~~- * Retruns the courses linked to this org.~~
	261	+ * Retruns the master courses linked to this org.
262	262	*
263	263	* @since 0.1
264	264	*
265	265	* @param array\|null $fields
266	266	*
267		~~- * @return array of EPCourse~~
	267	+ * @return array of EPMC
268	268	*/
269		~~- public function getCourses( array $fields = null ) {~~
270		~~- if ( $this->courses === false ) {~~
271		~~- $this->courses = EPCourse::select( $fields, array( 'org_id' => $this->getId() ) );~~
	269	+ public function getMasterCourses( array $fields = null ) {
	270	+ if ( $this->mcs === false ) {
	271	+ $this->mcs = EPMC::select( $fields, array( 'org_id' => $this->getId() ) );
272	272	}
273	273
274		~~- return $this->courses;~~
	274	+ return $this->mcs;
275	275	}
276	276
277	277	/**
278		~~- * Retruns the terms linked to this org.~~
	278	+ * Retruns the courses linked to this org.
279	279	*
280	280	* @since 0.1
281	281	*
282	282	* @param array\|null $fields
283	283	*
284		~~- * @return array of EPTerm~~
	284	+ * @return array of EPCourse
285	285	*/
286		~~- public function getTerms( array $fields = null ) {~~
287		~~- if ( $this->terms === false ) {~~
288		~~- $this->terms = EPTerm::select( $fields, array( 'org_id' => $this->getId() ) );~~
	286	+ public function getCourses( array $fields = null ) {
	287	+ if ( $this->courses === false ) {
	288	+ $this->courses = EPCourse::select( $fields, array( 'org_id' => $this->getId() ) );
289	289	}
290	290
291		~~- return $this->terms;~~
	291	+ return $this->courses;
292	292	}
293	293
294	294	/**
Index: trunk/extensions/EducationProgram/EducationProgram.php
—	—	@@ -148,7 +148,9 @@
149	149	$egEPDBObjects['EPMC'] = array( 'table' => 'ep_mcs', 'prefix' => 'mc_' );
150	150	$egEPDBObjects['EPCourse'] = array( 'table' => 'ep_courses', 'prefix' => 'course_' );
151	151	$egEPDBObjects['EPStudent'] = array( 'table' => 'ep_students', 'prefix' => 'student_' );
152		~~-$egEPDBObjects[] = array( 'table' => 'ep_students_per_term', 'prefix' => 'spt_' );~~
	152	+$egEPDBObjects['EPOA'] = array( 'table' => 'ep_oas', 'prefix' => 'oa_' );
	153	+$egEPDBObjects['EPCA'] = array( 'table' => 'ep_cas', 'prefix' => 'ca_' );
	154	+$egEPDBObjects[] = array( 'table' => 'ep_students_per_course', 'prefix' => 'spc_' );
153	155
154	156	// API
155	157	$wgAPIModules['deleteeducation'] = 'ApiDeleteEducation';
—	—	@@ -198,7 +200,7 @@
199	201	// Compatibility with MediaWiki 1.18.
200	202	$wgLogNames['institution'] = 'log-name-institution';
201	203	$wgLogNames['course'] = 'log-name-course';
202		~~- $wgLogNames['term'] = 'log-name-mc';~~
	204	+ $wgLogNames['mc'] = 'log-name-mc';
203	205	$wgLogNames['student'] = 'log-name-student';
204	206	$wgLogNames['ambassador'] = 'log-name-ambassador';
205	207	$wgLogNames['instructor'] = 'log-name-instructor';
—	—	@@ -206,7 +208,7 @@
207	209	// Compatibility with MediaWiki 1.18.
208	210	$wgLogHeaders['institution'] = 'log-header-institution';
209	211	$wgLogHeaders['course'] = 'log-header-course';
210		~~- $wgLogHeaders['term'] = 'log-header-mc';~~
	212	+ $wgLogHeaders['mc'] = 'log-header-mc';
211	213	$wgLogHeaders['student'] = 'log-header-student';
212	214	$wgLogHeaders['ambassador'] = 'log-header-ambassador';
213	215	$wgLogHeaders['instructor'] = 'log-header-instructor';

Past revisions this follows-up on

Revision	Commit summary	Author	Date
r109656	schema changes currently completely breaking the extension and added hook to ...	jeroendedauw	22:24, 20 January 2012

Status & tagging log

18:36, 21 January 2012 Aaron Schulz (talk | contribs) changed the status of r109667 [removed: new added: deferred]