r109070 MediaWiki - Code Review archive

Repository:	MediaWiki
Revision:	< r109069‎ \| r109070 \| r109071 >
Date:	22:27, 16 January 2012
Author:	jeroendedauw
Status:	deferred
Tags:
Comment:	work on instructor management
Modified paths:	/trunk/extensions/EducationProgram/EducationProgram.i18n.php (modified) (history) /trunk/extensions/EducationProgram/api/ApiInstructor.php (modified) (history) /trunk/extensions/EducationProgram/includes/EPCourse.php (modified) (history) /trunk/extensions/EducationProgram/resources/ep.api.js (modified) (history) /trunk/extensions/EducationProgram/resources/ep.instructor.js (modified) (history) /trunk/extensions/EducationProgram/specials/SpecialCourse.php (modified) (history)

Diff [purge]

Index: trunk/extensions/EducationProgram/specials/SpecialCourse.php
—	—	@@ -121,28 +121,81 @@
122	122	);
123	123	}
124	124
	125	+ $stats['instructors'] = $this->getInstructorsList( $course ) . $this->getInstructorControls( $course );
	126	+
	127	+ return $stats;
	128	+ }
	129	+
	130	+ /**
	131	+ * Returns a list with the instructors for the provided course
	132	+ * or a message indicating there are none.
	133	+ *
	134	+ * @since 0.1
	135	+ *
	136	+ * @param EPCourse $course
	137	+ *
	138	+ * @return string
	139	+ */
	140	+ protected function getInstructorsList( EPCourse $course ) {
125	141	$instructors = $course->getInstructors();
126	142
127	143	if ( count( $instructors ) > 0 ) {
128		~~- $stats['instructors'] = array();~~
	144	+ $instList = array();
129	145
130	146	foreach ( $instructors as /* EPInstructor */ $instructor ) {
131		~~- $stats['instructors'][] = $instructor->getUserLink() . $instructor->getToolLinks( $this->getContext(), $course );~~
	147	+ $instList[] = $instructor->getUserLink() . $instructor->getToolLinks( $this->getContext(), $course );
132	148	}
133	149
134	150	if ( count( $instructors ) == 1 ) {
135		~~- $stats['instructors'] = $stats['instructors'][0];~~
	151	+ return $instList[0];
136	152	}
137	153	else {
138		~~- $stats['instructors'] = '<ul><li>' . implode( '</li><li>', $stats['instructors'] ) . '</li></ul>';~~
	154	+ return '<ul><li>' . implode( '</li><li>', $instList ) . '</li></ul>';
139	155	}
140	156	}
141	157	else {
142		~~- $stats['instructors'] = wfMsgHtml( 'ep-course-no-instructors' );~~
	158	+ return wfMsgHtml( 'ep-course-no-instructors' );
143	159	}
	160	+ }
	161	+
	162	+ protected function getInstructorControls( EPCourse $course ) {
	163	+ $user = $this->getUser();
	164	+ $links = array();
144	165
145		-
146		~~- return $stats;~~
	166	+ if ( ( $user->isAllowed( 'ep-instructor' ) \|\| $user->isAllowed( 'ep-beinstructor' ) )
	167	+ && !in_array( $user->getId(), $course->getField( 'instructors' ) )
	168	+ ) {
	169	+ $links[] = Html::element(
	170	+ 'a',
	171	+ array(
	172	+ 'href' => '#',
	173	+ 'class' => 'ep-become-instructor',
	174	+ 'data-courseid' => $course->getId(),
	175	+ 'data-coursename' => $course->getField( 'name' ),
	176	+ ),
	177	+ wfMsg( 'ep-course-become-instructor' )
	178	+ );
	179	+ }
	180	+
	181	+ if ( $user->isAllowed( 'ep-instructor' ) ) {
	182	+ $links[] = Html::element(
	183	+ 'a',
	184	+ array(
	185	+ 'href' => '#',
	186	+ 'class' => 'ep-add-instructor',
	187	+ 'data-courseid' => $course->getId(),
	188	+ 'data-coursename' => $course->getField( 'name' ),
	189	+ ),
	190	+ wfMsg( 'ep-course-add-instructor' )
	191	+ );
	192	+ }
	193	+
	194	+ if ( count( $links ) > 0 ) {
	195	+ return '<br />' . $this->getLanguage()->pipeList( $links );
	196	+ }
	197	+ else {
	198	+ return '';
	199	+ }
147	200	}
148	201
149	202	}
Index: trunk/extensions/EducationProgram/includes/EPCourse.php
—	—	@@ -412,9 +412,9 @@
413	413	}
414	414	}
415	415
416		~~- $this->setField( 'instructors', $instructors );~~
417		-
418	416	if ( count( $addedInstructors ) > 0 ) {
	417	+ $this->setField( 'instructors', $instructors );
	418	+
419	419	$success = true;
420	420
421	421	if ( $save ) {
—	—	@@ -424,27 +424,7 @@
425	425	}
426	426
427	427	if ( $success && $log ) {
428		~~- $names = array();~~
429		-
430		~~- foreach ( $addedInstructors as $userId ) {~~
431		~~- $names[] = EPInstructor::newFromId( $userId )->getName();~~
432		~~- }~~
433		-
434		~~- $info = array(~~
435		~~- 'type' => 'instructor',~~
436		~~- 'subtype' => 'add',~~
437		~~- 'title' => $this->getTitle(),~~
438		~~- 'parameters' => array(~~
439		~~- '4::instructorcount' => count( $names ),~~
440		~~- '5::instructors' => $GLOBALS['wgLang']->listToText( $names )~~
441		~~- ),~~
442		~~- );~~
443		-
444		~~- if ( $message !== '' ) {~~
445		~~- $info['comment'] = $message;~~
446		~~- }~~
447		-
448		~~- EPUtils::log( $info );~~
	428	+ $this->logInstructorChange( 'add', $addedInstructors, $message );
449	429	}
450	430
451	431	return $success;
—	—	@@ -469,7 +449,72 @@
470	450	* @return boolean Success indicator
471	451	*/
472	452	public function removeInstructors( $sadInstructors, $message = '', $save = true, $log = true ) {
473		~~- // TODO~~
	453	+ $removedInstructors = array();
	454	+ $remaimingInstructors = array();
	455	+ $sadInstructors = (array)$sadInstructors;
	456	+
	457	+ foreach ( $this->getField( 'instructors' ) as $userId ) {
	458	+ if ( in_array( $userId, $sadInstructors ) ) {
	459	+ $removedInstructors[] = $userId;
	460	+ }
	461	+ else {
	462	+ $remaimingInstructors[] = $userId;
	463	+ }
	464	+ }
	465	+
	466	+ if ( count( $removedInstructors ) > 0 ) {
	467	+ $this->setField( 'instructors', $remaimingInstructors );
	468	+
	469	+ $success = true;
	470	+
	471	+ if ( $save ) {
	472	+ $this->disableLogging();
	473	+ $success = $this->writeToDB();
	474	+ $this->enableLogging();
	475	+ }
	476	+
	477	+ if ( $success && $log ) {
	478	+ $this->logInstructorChange( 'remove', $removedInstructors, $message );
	479	+ }
	480	+
	481	+ return $success;
	482	+ }
	483	+ else {
	484	+ return true;
	485	+ }
474	486	}
475	487
	488	+ /**
	489	+ * Log a change of the instructors of the course.
	490	+ *
	491	+ * @since 0.1
	492	+ *
	493	+ * @param string $action
	494	+ * @param array $instructors
	495	+ * @param string $message
	496	+ */
	497	+ protected function logInstructorChange( $action, array $instructors, $message ) {
	498	+ $names = array();
	499	+
	500	+ foreach ( $instructors as $userId ) {
	501	+ $names[] = EPInstructor::newFromId( $userId )->getName();
	502	+ }
	503	+
	504	+ $info = array(
	505	+ 'type' => 'instructor',
	506	+ 'subtype' => $action,
	507	+ 'title' => $this->getTitle(),
	508	+ 'parameters' => array(
	509	+ '4::instructorcount' => count( $names ),
	510	+ '5::instructors' => $GLOBALS['wgLang']->listToText( $names )
	511	+ ),
	512	+ );
	513	+
	514	+ if ( $message !== '' ) {
	515	+ $info['comment'] = $message;
	516	+ }
	517	+
	518	+ EPUtils::log( $info );
	519	+ }
	520	+
476	521	}
Index: trunk/extensions/EducationProgram/EducationProgram.i18n.php
—	—	@@ -308,6 +308,8 @@
309	309	'specialcourse-summary-terms' => 'Term count',
310	310	'specialcourse-summary-instructors' => 'Instructors',
311	311	'ep-course-no-instructors' => 'There are no instructors for this course.',
	312	+ 'ep-course-become-instructor' => 'Become instructor',
	313	+ 'ep-course-add-instructor' => 'Add an instructor',
312	314
313	315	// Special:Term
314	316	'ep-term-title' => 'Term: $1',
Index: trunk/extensions/EducationProgram/api/ApiInstructor.php
—	—	@@ -47,10 +47,10 @@
48	48
49	49	switch ( $params['subaction'] ) {
50	50	case 'add':
51		~~- $success = $course->addInstructors( array( $userId ) );~~
	51	+ $success = $course->addInstructors( array( $userId ), $params['reason'] );
52	52	break;
53	53	case 'remove':
54		~~- $success = $course->removeInstructors( array( $userId ) );~~
	54	+ $success = $course->removeInstructors( array( $userId ), $params['reason'] );
55	55	break;
56	56	}
57	57
—	—	@@ -123,6 +123,7 @@
124	124	'reason' => array(
125	125	ApiBase::PARAM_TYPE => 'string',
126	126	ApiBase::PARAM_REQUIRED => false,
	127	+ ApiBase::PARAM_DFLT => '',
127	128	),
128	129	'token' => null,
129	130	);
Index: trunk/extensions/EducationProgram/resources/ep.api.js
—	—	@@ -32,10 +32,9 @@
33	33	);
34	34	};
35	35
36		~~- this.addInstructor = function( args ) {~~
	36	+ this.instructor = function( args ) {
37	37	var requestArgs = $.extend( {
38	38	'action': 'instructor',
39		~~- 'subaction': 'add',~~
40	39	'format': 'json',
41	40	'token': window.mw.user.tokens.get( 'editToken' )
42	41	}, args );
—	—	@@ -57,6 +56,16 @@
58	57
59	58	return deferred.promise();
60	59	};
	60	+
	61	+ this.addInstructor = function( args ) {
	62	+ args.subaction = 'add';
	63	+ return this.instructor( args );
	64	+ };
	65	+
	66	+ this.removeInstructor = function( args ) {
	67	+ args.subaction = 'remove';
	68	+ return this.instructor( args );
	69	+ };
61	70
62	71	} );
63	72
Index: trunk/extensions/EducationProgram/resources/ep.instructor.js
—	—	@@ -26,7 +26,7 @@
27	27	$remove.button( 'option', 'disabled', true );
28	28	$remove.button( 'option', 'label', mw.msg( 'ep-instructor-removing' ) );
29	29
30		~~- ep.api.addInstructor( {~~
	30	+ ep.api.removeInstructor( {
31	31	'courseid': courseId,
32	32	'userid': userId,
33	33	'reason': summaryInput.val()
—	—	@@ -67,7 +67,12 @@
68	68	]
69	69	} );
70	70
71		~~- $dialog.append( $( '<p>' ).text( gM( 'ep-instructor-remove-text', userName, bestName, courseName ) ) );~~
	71	+ $dialog.append( $( '<p>' ).msg(
	72	+ 'ep-instructor-remove-text',
	73	+ mw.html.escape( userName ),
	74	+ '<b>' + mw.html.escape( bestName ) + '</b>',
	75	+ '<b>' + mw.html.escape( courseName ) + '</b>'
	76	+ ) );
72	77
73	78	$dialog.append( summaryInput );
74	79

Status & tagging log

05:34, 17 January 2012 Aaron Schulz (talk | contribs) changed the status of r109070 [removed: new added: deferred]