r113687 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r113686‎ | r113687 | r113688 >
Date:23:58, 12 March 2012
Author:jeroendedauw
Status:deferred
Tags:
Comment:
allow certain people to delete article associations from students
Modified paths:
  • /trunk/extensions/EducationProgram/EducationProgram.i18n.php (modified) (history)
  • /trunk/extensions/EducationProgram/EducationProgram.php (modified) (history)
  • /trunk/extensions/EducationProgram/actions/EPAddArticleAction.php (modified) (history)
  • /trunk/extensions/EducationProgram/actions/EPAddReviewerAction.php (modified) (history)
  • /trunk/extensions/EducationProgram/actions/EPRemoveArticleAction.php (modified) (history)
  • /trunk/extensions/EducationProgram/actions/EPRemoveReviewerAction.php (modified) (history)
  • /trunk/extensions/EducationProgram/includes/EPArticle.php (modified) (history)
  • /trunk/extensions/EducationProgram/includes/EPArticleTable.php (modified) (history)
  • /trunk/extensions/EducationProgram/includes/EPLogFormatter.php (modified) (history)

Diff [purge]

Index: trunk/extensions/EducationProgram/EducationProgram.php
@@ -11,6 +11,7 @@
1212 * * Org instead of Institution
1313 * * CA for campus ambassador
1414 * * OA for online ambassador
 15+ * * Article is often used to refer to "article student associations" rather then the Article class.
1516 *
1617 * @file EducationProgram.php
1718 * @ingroup EducationProgram
@@ -241,7 +242,9 @@
242243 $wgAvailableRights[] = 'ep-remreviewer'; // Remove reviewers from articles
243244 $wgAvailableRights[] = 'ep-bulkdelorgs'; // Bulk delete institutions
244245 $wgAvailableRights[] = 'ep-bulkdelcourses'; // Bulk delete courses
 246+$wgAvailableRights[] = 'ep-remarticle'; // Remove artiles (from being student associated)
245247
 248+
246249 // User group rights
247250 $wgGroupPermissions['*']['ep-enroll'] = true;
248251 $wgGroupPermissions['*']['ep-org'] = false;
@@ -254,10 +257,11 @@
255258 $wgGroupPermissions['*']['ep-beonline'] = false;
256259 $wgGroupPermissions['*']['ep-becampus'] = false;
257260 $wgGroupPermissions['*']['ep-beinstructor'] = false;
258 -$wgGroupPermissions['*']['ep-bereviewer'] = true; // TODO: do we want this?
 261+$wgGroupPermissions['*']['ep-bereviewer'] = true;
259262 $wgGroupPermissions['*']['ep-remreviewer'] = false;
260263 $wgGroupPermissions['*']['ep-bulkdelorgs'] = false;
261264 $wgGroupPermissions['*']['ep-bulkdelcourses'] = false;
 265+$wgGroupPermissions['*']['ep-remarticle'] = false;
262266
263267 $wgGroupPermissions['epstaff']['ep-org'] = true;
264268 $wgGroupPermissions['epstaff']['ep-course'] = true;
@@ -274,6 +278,7 @@
275279 $wgGroupPermissions['epstaff']['ep-remreviewer'] = true;
276280 $wgGroupPermissions['epstaff']['ep-bulkdelorgs'] = true;
277281 $wgGroupPermissions['epstaff']['ep-bulkdelcourses'] = true;
 282+$wgGroupPermissions['epstaff']['ep-remarticle'] = true;
278283
279284 $wgGroupPermissions['epadmin']['ep-org'] = true;
280285 $wgGroupPermissions['epadmin']['ep-course'] = true;
@@ -289,22 +294,26 @@
290295 $wgGroupPermissions['epadmin']['ep-bereviewer'] = true;
291296 $wgGroupPermissions['epadmin']['ep-remreviewer'] = true;
292297 $wgGroupPermissions['epadmin']['ep-bulkdelcourses'] = true;
 298+$wgGroupPermissions['epadmin']['ep-remarticle'] = true;
293299
294300 $wgGroupPermissions['eponlineamb']['ep-org'] = true;
295301 $wgGroupPermissions['eponlineamb']['ep-course'] = true;
296302 $wgGroupPermissions['eponlineamb']['ep-token'] = true;
297303 $wgGroupPermissions['eponlineamb']['ep-beonline'] = true;
 304+$wgGroupPermissions['eponlineamb']['ep-remarticle'] = true;
298305
299306 $wgGroupPermissions['epcampamb']['ep-org'] = true;
300307 $wgGroupPermissions['epcampamb']['ep-course'] = true;
301308 $wgGroupPermissions['epcampamb']['ep-token'] = true;
302309 $wgGroupPermissions['epcampamb']['ep-becampus'] = true;
 310+$wgGroupPermissions['epcampamb']['ep-remarticle'] = true;
303311
304312 $wgGroupPermissions['epinstructor']['ep-org'] = true;
305313 $wgGroupPermissions['epinstructor']['ep-course'] = true;
306314 $wgGroupPermissions['epinstructor']['ep-token'] = true;
307315 $wgGroupPermissions['epinstructor']['ep-beinstructor'] = true;
308316 $wgGroupPermissions['epinstructor']['ep-remstudent'] = true;
 317+$wgGroupPermissions['epinstructor']['ep-remarticle'] = true;
309318
310319 $wgGroupPermissions['epstaff']['userrights'] = false;
311320 $wgAddGroups['epstaff'] = array( 'epstaff', 'epadmin', 'eponlineamb', 'epcampamb', 'epinstructor' );
Index: trunk/extensions/EducationProgram/actions/EPRemoveReviewerAction.php
@@ -37,7 +37,7 @@
3838 && ( $user->getId() === $userIdToRemove || $user->isAllowed( 'ep-remreviewer' ) ) ) {
3939
4040 $article = EPArticles::singleton()->selectRow(
41 - array( 'id', 'reviewers' ),
 41+ null,
4242 array( 'id' => $req->getInt( 'article-id' ) )
4343 );
4444
Index: trunk/extensions/EducationProgram/actions/EPAddArticleAction.php
@@ -53,15 +53,7 @@
5454 $article = EPArticles::singleton()->newFromArray( $articleData, true );
5555
5656 if ( $article->save() ) {
57 - EPUtils::log( array(
58 - 'type' => 'eparticle',
59 - 'subtype' => 'add',
60 - 'user' => $this->getUser(),
61 - 'title' => $course->getTitle(),
62 - 'parameters' => array(
63 - '4::articlename' => $title->getFullText(),
64 - ),
65 - ) );
 57+ $article->logAdittion( $this->getUser() );
6658 }
6759 }
6860 }
Index: trunk/extensions/EducationProgram/actions/EPAddReviewerAction.php
@@ -36,7 +36,7 @@
3737 if ( $user->matchEditToken( $req->getText( 'token' ), $salt ) ) {
3838
3939 $article = EPArticles::singleton()->selectRow(
40 - array( 'id', 'reviewers', 'page_id', 'page_title' ),
 40+ null,
4141 array( 'id' => $req->getInt( 'article-id' ) )
4242 );
4343
Index: trunk/extensions/EducationProgram/actions/EPRemoveArticleAction.php
@@ -32,28 +32,14 @@
3333
3434 if ( $user->matchEditToken( $req->getText( 'token' ), 'remarticle' . $req->getInt( 'article-id' ) ) ) {
3535 $article = EPArticles::singleton()->selectRow(
 36+ null,
3637 array(
37 - 'id',
38 - 'course_id',
39 - 'page_id',
40 - 'page_title',
41 - ),
42 - array(
4338 'id' => $req->getInt( 'article-id' ),
44 - 'user_id' => $user->getId(),
4539 )
4640 );
4741
48 - if ( $article !== false && $article->remove() ) {
49 - EPUtils::log( array(
50 - 'type' => 'eparticle',
51 - 'subtype' => 'remove',
52 - 'user' => $this->getUser(),
53 - 'title' => $article->getCourse()->getTitle(),
54 - 'parameters' => array(
55 - '4::articlename' => $article->getTitle()->getFullText(),
56 - ),
57 - ) );
 42+ if ( $article !== false && $article->userCanRemove( $this->getUser() ) && $article->remove() ) {
 43+ $article->logRemoval( $this->getUser() );
5844 }
5945 }
6046
Index: trunk/extensions/EducationProgram/includes/EPLogFormatter.php
@@ -21,6 +21,11 @@
2222 *
2323 * This is overridden to change the link text to only include the name of the object,
2424 * rather then the full name of it's page.
 25+ *
 26+ * @param Title $title
 27+ * @param $parameters
 28+ *
 29+ * @return string
2530 */
2631 protected function makePageLink( Title $title = null, $parameters = array() ) {
2732 if ( !$title instanceof Title ) {
@@ -41,6 +46,17 @@
4247
4348 }
4449
 50+/**
 51+ * Class for logging role changes. ie people gaining or losing a role.
 52+ *
 53+ * @since 0.1
 54+ *
 55+ * @file EPLogFormatter.php
 56+ * @ingroup EducationProgram
 57+ *
 58+ * @licence GNU GPL v3 or later
 59+ * @author Jeroen De Dauw < jeroendedauw@gmail.com >
 60+ */
4561 class EPRoleChangeFormatter extends EPLogFormatter {
4662
4763 /**
@@ -62,6 +78,17 @@
6379
6480 }
6581
 82+/**
 83+ * Class for logging role changes to student article associations.
 84+ *
 85+ * @since 0.1
 86+ *
 87+ * @file EPLogFormatter.php
 88+ * @ingroup EducationProgram
 89+ *
 90+ * @licence GNU GPL v3 or later
 91+ * @author Jeroen De Dauw < jeroendedauw@gmail.com >
 92+ */
6693 class EPArticleFormatter extends EPLogFormatter {
6794
6895 /**
@@ -73,6 +100,12 @@
74101
75102 if ( !empty( $params ) ) {
76103 $params[3] = Message::rawParam( Linker::link( Title::newFromText( $params[3] ) ) );
 104+
 105+ if ( isset( $params[4] ) ) {
 106+ list( $id, $name ) = $params[4];
 107+ $params[4] = Message::rawParam( Linker::userLink( $id, $name ) );
 108+ $params[5] = $name;
 109+ }
77110 }
78111
79112 return $params;
Index: trunk/extensions/EducationProgram/includes/EPArticleTable.php
@@ -310,7 +310,7 @@
311311 'returnto' => $this->getTitle()->getFullText(),
312312 ) );
313313
314 - if ( $this->getUser()->getId() === $article->getField( 'user_id' ) ) {
 314+ if ( $article->userCanRemove( $this->getUser() ) ) {
315315 $html .= ' (' . Html::element(
316316 'a',
317317 $attr,
Index: trunk/extensions/EducationProgram/includes/EPArticle.php
@@ -177,12 +177,7 @@
178178 */
179179 public function logReviewersAdittion( array $userIds ) {
180180 foreach ( $userIds as $userId ) {
181 - EPUtils::log( array(
182 - 'user' => User::newFromId( $userId ),
183 - 'title' => $this->getTitle(),
184 - 'type' => 'eparticle',
185 - 'subtype' => 'review',
186 - ) );
 181+ $this->log( User::newFromId( $userId ), 'review' );
187182 }
188183 }
189184
@@ -216,13 +211,68 @@
217212 */
218213 public function logReviewersRemoval( array $userIds ) {
219214 foreach ( $userIds as $userId ) {
220 - EPUtils::log( array(
221 - 'user' => User::newFromId( $userId ),
222 - 'title' => $this->getTitle(),
223 - 'type' => 'eparticle',
224 - 'subtype' => 'unreview',
225 - ) );
 215+ $this->log( User::newFromId( $userId ), 'unreview' );
226216 }
227217 }
228218
 219+ /**
 220+ * Log adittion of the article.
 221+ *
 222+ * @param User $actionUser
 223+ */
 224+ public function logAdittion( User $actionUser ) {
 225+ $this->log(
 226+ $actionUser,
 227+ $actionUser->getId() === $this->getUser()->getId() ? 'selfadd' : 'add'
 228+ );
 229+ }
 230+
 231+ /**
 232+ * Log removal of the article.
 233+ *
 234+ * @param User $actionUser
 235+ */
 236+ public function logRemoval( User $actionUser ) {
 237+ $this->log(
 238+ $actionUser,
 239+ $actionUser->getId() === $this->getUser()->getId() ? 'selfremove' : 'remove'
 240+ );
 241+ }
 242+
 243+ /**
 244+ * Logging helper method.
 245+ *
 246+ * @since 0.1
 247+ *
 248+ * @param User $actionUser
 249+ * @param string $subType
 250+ */
 251+ protected function log( User $actionUser, $subType ) {
 252+ $articleOwner = $this->getUser();
 253+
 254+ EPUtils::log( array(
 255+ 'user' => $actionUser,
 256+ 'title' => $this->getTitle(),
 257+ 'type' => 'eparticle',
 258+ 'subtype' => $subType,
 259+ 'parameters' => array(
 260+ '4::coursename' => $this->getCourse()->getTitle()->getFullText(),
 261+ '5::owner' => array( $articleOwner->getId(), $articleOwner->getName() ),
 262+ ),
 263+ ) );
 264+ }
 265+
 266+ /**
 267+ * Returns if the provided user can remove the article.
 268+ *
 269+ * @since 0.1
 270+ *
 271+ * @param User $user
 272+ *
 273+ * @return boolean
 274+ */
 275+ public function userCanRemove( User $user ) {
 276+ return $user->isAllowed( 'ep-remarticle' ) || $user->getId() === $this->getField( 'user_id' );
 277+ }
 278+
229279 }
Index: trunk/extensions/EducationProgram/EducationProgram.i18n.php
@@ -118,8 +118,12 @@
119119 'logentry-student-selfadd' => '$1 enrolled in course $3',
120120 'logentry-student-selfremove' => '$1 disenrolled from course $3',
121121
122 - 'logentry-eparticle-add' => '$1 added article $4 to {{GENDER:$2|his|her}} list of articles at course $3',
123 - 'logentry-eparticle-remove' => '$1 removed article $4 from {{GENDER:$2|his|her}} list of articles at course $3',
 122+ 'logentry-eparticle-selfadd' => '$1 added article $3 to {{GENDER:$2|his|her}} list of articles at course $4',
 123+ 'logentry-eparticle-selfremove' => '$1 removed article $3 from {{GENDER:$2|his|her}} list of articles at course $4',
 124+ 'logentry-eparticle-add' => '$1 added article $3 to $5 {{GENDER:$6|his|her}} list of articles at course $4',
 125+ 'logentry-eparticle-remove' => '$1 removed article $3 from $5 {{GENDER:$6|his|her}} list of articles at course $4',
 126+ 'logentry-eparticle-review' => '$1 added {{GENDER:$2|himself|herself}} as reviewer to article $3 worked upon by $5 as part of course $4',
 127+ 'logentry-eparticle-unreview' => '$1 removed {{GENDER:$2|himself|herself}} as reviewer to article $3 worked upon by $5 as part of course $4',
124128
125129 // Preferences
126130 'prefs-education' => 'Education',

Follow-up revisions

RevisionCommit summaryAuthorDate
r113694ui follow up to r113687jeroendedauw00:22, 13 March 2012

Status & tagging log