r107107 MediaWiki - Code Review archive

Repository:	MediaWiki
Revision:	< r107106‎ \| r107107 \| r107108 >
Date:	20:17, 22 December 2011
Author:	jeroendedauw
Status:	deferred (Comments)
Tags:	educationprogram
Comment:	finished up some stuff
Modified paths:	/trunk/extensions/EducationProgram/EducationProgram.i18n.php (modified) (history) /trunk/extensions/EducationProgram/includes/EPCoursePager.php (modified) (history) /trunk/extensions/EducationProgram/includes/EPOrgPager.php (modified) (history) /trunk/extensions/EducationProgram/includes/EPPager.php (modified) (history) /trunk/extensions/EducationProgram/includes/EPTermPager.php (modified) (history) /trunk/extensions/EducationProgram/specials/SpecialCourse.php (modified) (history) /trunk/extensions/EducationProgram/specials/SpecialCourses.php (modified) (history) /trunk/extensions/EducationProgram/specials/SpecialEditCourse.php (modified) (history) /trunk/extensions/EducationProgram/specials/SpecialInstitution.php (modified) (history) /trunk/extensions/EducationProgram/specials/SpecialTerm.php (modified) (history)

Diff [purge]

Index: trunk/extensions/EducationProgram/specials/SpecialInstitution.php
—	—	@@ -1,7 +1,8 @@
2	2	<?php
3	3
4	4	/**
5		- *
	5	+ * Shows the info for a single institution, with management and
	6	+ * enrollment controls depending on the user and his rights.
6	7	*
7	8	* @since 0.1
8	9	*
Index: trunk/extensions/EducationProgram/specials/SpecialCourse.php
—	—	@@ -1,7 +1,8 @@
2	2	<?php
3	3
4	4	/**
5		- *
	5	+ * Shows the info for a single course, with management and
	6	+ * enrollment controls depending on the user and his rights.
6	7	*
7	8	* @since 0.1
8	9	*
Index: trunk/extensions/EducationProgram/specials/SpecialCourses.php
—	—	@@ -59,15 +59,16 @@
60	60	protected function displayPage() {
61	61	$user = $this->getUser();
62	62
63		~~- if ( $user->isAllowed( 'epadmin' ) ) {~~
64		~~- $this->displayAddNewControl();~~
	63	+ $orgs = EPOrg::getEditableOrgs( $this->getUser() );
	64	+
	65	+ if ( count( $orgs ) > 0 ) {
	66	+ $this->displayAddNewControl( $orgs );
65	67	}
	68	+ elseif ( $user->isAllowed( 'epadmin' ) ) {
	69	+ $this->getOutput()->addWikiMsg( 'ep-courses-noorgs' );
	70	+ }
66	71	elseif ( $user->isAllowed( 'epmentor' ) ) {
67		~~- $mentor = EPMentor::select( array( 'user_id' => $user->getId() ) );~~
68		-
69		~~- if ( $mentor !== false && count( $mentor->getOrgs() ) > 0 ) {~~
70		~~- $this->displayAddNewControl();~~
71		~~- }~~
	72	+ $this->getOutput()->addWikiMsg( 'ep-courses-addorgfirst' );
72	73	}
73	74
74	75	$pager = new EPCoursePager( $this->getContext() );
—	—	@@ -90,8 +91,10 @@
91	92	* Displays a small form to add a new institution.
92	93	*
93	94	* @since 0.1
	95	+ *
	96	+ * @param array $orgs
94	97	*/
95		~~- protected function displayAddNewControl() {~~
	98	+ protected function displayAddNewControl( array /* of EPOrg */ $orgs ) {
96	99	$out = $this->getOutput();
97	100
98	101	$out->addHTML( Html::openElement(
—	—	@@ -108,9 +111,21 @@
109	112
110	113	$out->addHTML( Html::element( 'p', array(), wfMsg( 'ep-courses-namedoc' ) ) );
111	114
	115	+ $out->addHTML( Html::element( 'label', array( 'for' => 'neworg' ), wfMsg( 'ep-courses-neworg' ) ) );
	116	+
	117	+ $out->addHTML( ' ' );
	118	+
	119	+ $select = new XmlSelect( 'neworg', 'neworg' );
	120	+ $select->addOptions( EPOrg::getOrgOptions( $orgs ) );
	121	+ $out->addHTML( $select->getHTML() );
	122	+
	123	+ $out->addHTML( ' ' );
	124	+
112	125	$out->addHTML( Xml::inputLabel( wfMsg( 'ep-courses-newname' ), 'newname', 'newname' ) );
113	126
114		~~- $out->addHTML( ' ' . Html::input(~~
	127	+ $out->addHTML( ' ' );
	128	+
	129	+ $out->addHTML( Html::input(
115	130	'addneworg',
116	131	wfMsg( 'ep-courses-add' ),
117	132	'submit'
Index: trunk/extensions/EducationProgram/specials/SpecialEditCourse.php
—	—	@@ -64,4 +64,15 @@
65	65	return $this->processFormFields( $fields );
66	66	}
67	67
	68	+ /**
	69	+ * (non-PHPdoc)
	70	+ * @see SpecialEPFormPage::getNewData()
	71	+ */
	72	+ protected function getNewData() {
	73	+ return array(
	74	+ 'org_id' => $this->getRequest()->getVal( 'neworg' ),
	75	+ 'name' => $this->getRequest()->getVal( 'newname' ),
	76	+ );
	77	+ }
	78	+
68	79	}
Index: trunk/extensions/EducationProgram/specials/SpecialTerm.php
—	—	@@ -0,0 +1,41 @@
	2	+<?php
	3	+
	4	+/**
	5	+ * Shows the info for a single term, with management and
	6	+ * enrollment controls depending on the user and his rights.
	7	+ *
	8	+ * @since 0.1
	9	+ *
	10	+ * @file SpecialTerm.php
	11	+ * @ingroup EducationProgram
	12	+ *
	13	+ * @licence GNU GPL v3 or later
	14	+ * @author Jeroen De Dauw < jeroendedauw@gmail.com >
	15	+ */
	16	+class SpecialTerm extends SpecialEPPage {
	17	+
	18	+ /**
	19	+ * Constructor.
	20	+ *
	21	+ * @since 0.1
	22	+ */
	23	+ public function __construct() {
	24	+ parent::__construct( 'Term' );
	25	+ }
	26	+
	27	+ /**
	28	+ * Main method.
	29	+ *
	30	+ * @since 0.1
	31	+ *
	32	+ * @param string $arg
	33	+ */
	34	+ public function execute( $subPage ) {
	35	+ parent::execute( $subPage );
	36	+
	37	+ $out = $this->getOutput();
	38	+
	39	+ // TODO: AUTH
	40	+ }
	41	+
	42	+}
Index: trunk/extensions/EducationProgram/includes/EPOrgPager.php
—	—	@@ -20,9 +20,6 @@
21	21	* @param array $conds
22	22	*/
23	23	public function __construct( IContextSource $context, array $conds = array() ) {
24		~~- $this->mDefaultDirection = true;~~
25		-
26		~~- // when MW 1.19 becomes min, we want to pass an IContextSource $context here.~~
27	24	parent::__construct( $context, $conds, 'EPOrg' );
28	25	}
29	26
—	—	@@ -130,16 +127,7 @@
131	128	wfMsg( 'edit' )
132	129	);
133	130
134		~~- $links[] = $value = Html::element(~~
135		~~- 'a',~~
136		~~- array(~~
137		~~- 'href' => '#',~~
138		~~- 'class' => 'ep-pager-delete',~~
139		~~- 'data-id' => $item->getId(),~~
140		~~- 'data-type' => 'org',~~
141		~~- ),~~
142		~~- wfMsg( 'delete' )~~
143		~~- );~~
	131	+ $links[] = $this->getDeletionLink( 'org', $item->getId() );
144	132	}
145	133
146	134	return $links;
Index: trunk/extensions/EducationProgram/includes/EPTermPager.php
—	—	@@ -20,9 +20,6 @@
21	21	* @param array $conds
22	22	*/
23	23	public function __construct( IContextSource $context, array $conds = array() ) {
24		~~- $this->mDefaultDirection = true;~~
25		-
26		~~- // when MW 1.19 becomes min, we want to pass an IContextSource $context here.~~
27	24	parent::__construct( $context, $conds, 'EPTerm' );
28	25	}
29	26
—	—	@@ -142,11 +139,18 @@
143	140	$links = parent::getControlLinks( $item );
144	141
145	142	$links[] = $value = Linker::linkKnown(
146		~~- SpecialPage::getTitleFor( 'EditTerm', $item->getId() ),~~
147		~~- wfMsg( 'edit' )~~
	143	+ SpecialPage::getTitleFor( 'Term', $item->getId() ),
	144	+ wfMsg( 'view' )
148	145	);
149	146
150		~~- // TODO~~
	147	+ if ( $this->getUser()->isAllowed( 'epadmin' ) ) {
	148	+ $links[] = $value = Linker::linkKnown(
	149	+ SpecialPage::getTitleFor( 'EditTerm', $item->getId() ),
	150	+ wfMsg( 'edit' )
	151	+ );
	152	+
	153	+ $links[] = $this->getDeletionLink( 'term', $item->getId() );
	154	+ }
151	155
152	156	return $links;
153	157	}
Index: trunk/extensions/EducationProgram/includes/EPPager.php
—	—	@@ -54,6 +54,8 @@
55	55	$this->className = $className;
56	56	$this->context = $context;
57	57
	58	+ $this->mDefaultDirection = true;
	59	+
58	60	if ( version_compare( $GLOBALS['wgVersion'], '1.18c', '>' ) ) {
59	61	parent::__construct( $context );
60	62	}
—	—	@@ -377,5 +379,28 @@
378	380	protected function getControlLinks( EPDBObject $item ) {
379	381	return array();
380	382	}
	383	+
	384	+ /**
	385	+ * Returns a deletion link.
	386	+ *
	387	+ * @since 0.1
	388	+ *
	389	+ * @param string $type
	390	+ * @paran integer $id
	391	+ *
	392	+ * @return string
	393	+ */
	394	+ protected function getDeletionLink( $type, $id ) {
	395	+ return Html::element(
	396	+ 'a',
	397	+ array(
	398	+ 'href' => '#',
	399	+ 'class' => 'ep-pager-delete',
	400	+ 'data-id' => $id,
	401	+ 'data-type' => $type,
	402	+ ),
	403	+ wfMsg( 'delete' )
	404	+ );
	405	+ }
381	406
382	407	}
Index: trunk/extensions/EducationProgram/includes/EPCoursePager.php
—	—	@@ -20,9 +20,6 @@
21	21	* @param array $conds
22	22	*/
23	23	public function __construct( IContextSource $context, array $conds = array() ) {
24		~~- $this->mDefaultDirection = true;~~
25		-
26		~~- // when MW 1.19 becomes min, we want to pass an IContextSource $context here.~~
27	24	parent::__construct( $context, $conds, 'EPCourse' );
28	25	}
29	26
—	—	@@ -121,11 +118,18 @@
122	119	$links = parent::getControlLinks( $item );
123	120
124	121	$links[] = $value = Linker::linkKnown(
125		~~- SpecialPage::getTitleFor( 'EditCourse', $item->getField( 'name' ) ),~~
126		~~- wfMsg( 'edit' )~~
	122	+ SpecialPage::getTitleFor( 'Course', $item->getField( 'name' ) ),
	123	+ wfMsg( 'view' )
127	124	);
128	125
129		~~- // TODO~~
	126	+ if ( $this->getUser()->isAllowed( 'epadmin' ) ) {
	127	+ $links[] = $value = Linker::linkKnown(
	128	+ SpecialPage::getTitleFor( 'EditCourse', $item->getField( 'name' ) ),
	129	+ wfMsg( 'edit' )
	130	+ );
	131	+
	132	+ $links[] = $this->getDeletionLink( 'term', $item->getId() );
	133	+ }
130	134
131	135	return $links;
132	136	}
Index: trunk/extensions/EducationProgram/EducationProgram.i18n.php
—	—	@@ -77,6 +77,9 @@
78	78	'ep-courses-namedoc' => 'Enter the name for the new course (which should be unique) and hit the button.',
79	79	'ep-courses-newname' => 'Course name:',
80	80	'ep-courses-add' => 'Add course',
	81	+ 'ep-courses-addorgfirst' => 'You need to [[Special:Institutions\|add an institution]] before you can create any courses.',
	82	+ 'ep-courses-noorgs' => 'You are not a mentor of any insitutions yet, so cannot add any courses.',
	83	+ 'ep-courses-neworg' => 'Institution',
81	84
82	85	// Special:Terms
83	86	'ep-terms-nosuchterm' => 'There is no term with id "$1". Existing terms are listed below.',

Follow-up revisions

Revision	Commit summary	Author	Date
r107179	restructuring some stuff and initial work on display of cousres, orgs and ter...	jeroendedauw	20:40, 23 December 2011

Comments

#Comment by Nikerabbit (talk | contribs) 09:04, 23 December 2011

+			$links[] = $value = Linker::linkKnown(
+				SpecialPage::getTitleFor( 'EditTerm', $item->getId() ),
+				wfMsg( 'edit' )
+			);

$value looks unused. Should not reuse messages. The message should be escaped. Two instances.

#Comment by Jeroen De Dauw (talk | contribs) 18:49, 23 December 2011

Good catch, did not realize I had to escape this arg (even though it's sort of obvious), and so did not do that anywhere in the extension :) Will be fixed in next commit.

Status & tagging log

11:13, 5 January 2012 Hashar (talk | contribs) changed the status of r107107 [removed: new added: deferred]
21:34, 23 December 2011 Jeroen De Dauw (talk | contribs) changed the status of r107107 [removed: fixme added: new]
09:04, 23 December 2011 Nikerabbit (talk | contribs) changed the status of r107107 [removed: new added: fixme]
03:08, 23 December 2011 RobLa-WMF (talk | contribs) changed the tags for r107107 [added: educationprogram]