CSteipp has submitted this change and it was merged.
Change subject: Remove code redundancies and centralize logging logic
......................................................................
Remove code redundancies and centralize logging logic
This change removes even more code redundancies from
CentralAuth and centralizes the logging logic in
CentralAuthUser.
Change-Id: Idab50ad6e61f210fede7b321531fc7596fdc1b77
---
M CentralAuthUser.php
M api/ApiDeleteGlobalAccount.php
M api/ApiSetGlobalAccountStatus.php
M specials/SpecialCentralAuth.php
M specials/SpecialMultiLock.php
5 files changed, 51 insertions(+), 120 deletions(-)
Approvals:
CSteipp: Verified; Looks good to me, approved
jenkins-bot: Checked
diff --git a/CentralAuthUser.php b/CentralAuthUser.php
index b981fc4..e776703 100644
--- a/CentralAuthUser.php
+++ b/CentralAuthUser.php
@@ -981,11 +981,12 @@
}
/**
- * Delete a global account
+ * Delete a global account and log what happened
*
+ * @reason string Reason for the deletion
* @return Status
*/
- function adminDelete() {
+ function adminDelete( $reason ) {
global $wgMemc;
wfDebugLog( 'CentralAuth', "Deleting global account for user
{$this->mName}" );
$centralDB = self::getCentralDB();
@@ -1022,6 +1023,7 @@
$centralDB->delete( 'localuser', array( 'lu_name' =>
$this->mName ), __METHOD__ );
$centralDB->commit();
+ $this->logAction( 'delete', $reason );
$this->invalidateCache();
return Status::newGood();
@@ -1092,9 +1094,7 @@
}
/**
- * Set locking and hiding settings for a Global User. This code was
copied from
- * SpecialCentralAuth.php in the hopes that all special and api pages
can use a
- * common function.
+ * Set locking and hiding settings for a Global User and log the
changes made.
*
* @param $lock Bool|null
* true = lock
@@ -1196,6 +1196,13 @@
$returnStatus->successCount = count( $added ) + count(
$removed );
$returnStatus->success['added'] = $added;
$returnStatus->success['removed'] = $removed;
+
+ $this->logAction(
+ 'setstatus',
+ $reason,
+ $returnStatus->success,
+ $setHidden != self::HIDDEN_NONE
+ );
} elseif ( !$good ) {
if ( !is_null( $lockStatus ) && !$lockStatus->isGood()
) {
@@ -2356,4 +2363,19 @@
$this->loadState( $recache );
return md5( $this->mGlobalId . ':' . $this->mName . ':' .
$this->mHidden . ':' . (int) $this->mLocked );
}
+
+ /**
+ * Log an action for the current user
+ *
+ * @param $action
+ * @param $reason string
+ * @param $params array
+ * @param $suppressLog bool
+ */
+ function logAction( $action, $reason = '', $params = array(),
$suppressLog = false ) {
+ // Not centralauth because of some weird length limitiations
+ $logType = $suppressLog ? 'suppress' : 'globalauth';
+ $log = new LogPage( $logType );
+ $log->addEntry( $action, Title::newFromText(
"User:{$this->mName}@global" ), $reason, $params );
+ }
}
diff --git a/api/ApiDeleteGlobalAccount.php b/api/ApiDeleteGlobalAccount.php
index 9ba03e6..dfb5491 100644
--- a/api/ApiDeleteGlobalAccount.php
+++ b/api/ApiDeleteGlobalAccount.php
@@ -43,10 +43,8 @@
$this->dieUsageMsg( array( 'nosuchuser',
$globalUser->getName() ) );
}
- $status = $globalUser->adminDelete();
+ $status = $globalUser->adminDelete( $params['reason'] );
if ( $status->isGood() ) {
- $sca = new SpecialCentralAuth;
- $sca->logAction( 'delete', $globalUser->getName(),
$params['reason'] );
$this->getResult()->addValue( null,
$this->getModuleName(), array(
'user' => $globalUser->getName(),
'reason' => $params['reason']
diff --git a/api/ApiSetGlobalAccountStatus.php
b/api/ApiSetGlobalAccountStatus.php
index d238033..c1bcc3e 100644
--- a/api/ApiSetGlobalAccountStatus.php
+++ b/api/ApiSetGlobalAccountStatus.php
@@ -42,83 +42,32 @@
$this->dieUsage( "At least one of the parameters
locked, hidden is required", "missingparam" );
}
- $setLocked = $this->getParameter( 'locked' ) == 'lock';
+ $setLocked = $this->getParameter( 'locked' );
+
+ if ( !$setLocked ) {
+ // Don't lock or unlock
+ $setLocked = null;
+ } else {
+ $setLocked = $setLocked === 'lock';
+ }
+
$setHidden = $this->getParameter( 'hidden' );
$reason = $this->getParameter( 'reason' );
$stateCheck = $this->getParameter( 'statecheck' );
- $isLocked = $globalUser->isLocked();
- $oldHiddenLevel = $globalUser->getHiddenLevel();
if ( $stateCheck && $stateCheck !== $globalUser->getStateHash(
true ) ) {
$this->dieUsage( "Edit conflict detected, Aborting." );
}
- if (
- $setHidden !== null && // hidden is set
- $oldHiddenLevel != $setHidden && // it's not the same
as the old hidden level
- !$this->getUser()->isAllowed( 'centralauth-oversight' )
// but the user doesn't have the right - oops!
- ) {
- $this->dieUsageMsg( array( 'badaccess-groups' ) );
- }
-
- $lockStatus = $hideStatus = null;
- $added = array();
- $removed = array();
-
- if ( !$isLocked && $setLocked ) {
- $lockStatus = $globalUser->adminLock();
- $added[] = $this->msg( 'centralauth-log-status-locked'
)->inContentLanguage()->text();
- } elseif ( $this->getRequest()->getCheck( 'locked' ) &&
$isLocked && !$setLocked ) { // Check that 'locked' is actually set, so that
hiding a locked user (without setting the locked parameter) doesn't unlock them.
- $lockStatus = $globalUser->adminUnlock();
- $removed[] = $this->msg(
'centralauth-log-status-locked' )->inContentLanguage()->text();
- }
-
- if ( $setHidden !== null && $oldHiddenLevel != $setHidden ) {
- $hideStatus = $globalUser->adminSetHidden( $setHidden );
- switch ( $setHidden ) {
- case CentralAuthUser::HIDDEN_NONE:
- $removed[] = $oldHiddenLevel ==
CentralAuthUser::HIDDEN_OVERSIGHT ?
- $this->msg(
'centralauth-log-status-oversighted' )->inContentLanguage()->text() :
- $this->msg(
'centralauth-log-status-hidden' )->inContentLanguage()->text();
- break;
- case CentralAuthUser::HIDDEN_LISTS:
- $added[] = $this->msg(
'centralauth-log-status-hidden' )->inContentLanguage()->text();
- if ( $oldHiddenLevel ==
CentralAuthUser::HIDDEN_OVERSIGHT )
- $removed[] = $this->msg(
'centralauth-log-status-oversighted' )->inContentLanguage()->text();
- break;
- case CentralAuthUser::HIDDEN_OVERSIGHT:
- $added[] = $this->msg(
'centralauth-log-status-oversighted' )->inContentLanguage()->text();
- if ( $oldHiddenLevel ==
CentralAuthUser::HIDDEN_LISTS )
- $removed[] = $this->msg(
'centralauth-log-status-hidden' )->inContentLanguage()->text();
- break;
- }
-
- if ( $setHidden == CentralAuthUser::HIDDEN_OVERSIGHT ) {
- $globalUser->suppress( $reason );
- } elseif ( $oldHiddenLevel ==
CentralAuthUser::HIDDEN_OVERSIGHT ) {
- $globalUser->unsuppress( $reason );
- }
- }
-
- $good =
- ( is_null( $lockStatus ) || $lockStatus->isGood() ) &&
- ( is_null( $hideStatus ) || $hideStatus->isGood() );
+ $status = $globalUser->adminLockHide(
+ $setLocked,
+ $setHidden,
+ $reason,
+ $this->getContext()
+ );
// Logging etc
- if ( $good && ( count( $added ) || count( $removed ) ) ) {
- $added = count( $added ) ?
- implode( ', ', $added ) : $this->msg(
'centralauth-log-status-none' )->inContentLanguage()->text();
- $removed = count( $removed ) ?
- implode( ', ', $removed ) : $this->msg(
'centralauth-log-status-none' )->inContentLanguage()->text();
-
- $sca = new SpecialCentralAuth;
- $sca->logAction(
- 'setstatus',
- $this->getParameter( 'user' ),
- $reason,
- array( $added, $removed ),
- $setHidden == CentralAuthUser::HIDDEN_OVERSIGHT
- );
+ if ( $status->isGood() ) {
$this->getResult()->addValue( null,
$this->getModuleName(), array(
'user' => $globalUser->getName(),
'locked' => $globalUser->isLocked(),
@@ -126,20 +75,11 @@
'reason' => $reason
) );
} else {
- if ( !is_null( $lockStatus ) && !$lockStatus->isGood()
) {
- $this->getResult()->addValue(
- 'error',
- null,
-
$this->getResult()->convertStatusToArray( $lockStatus )
- );
- }
- if ( !is_null( $hideStatus ) && !$hideStatus->isGood()
) {
- $this->getResult()->addValue(
- 'error',
- null,
-
$this->getResult()->convertStatusToArray( $hideStatus )
- );
- }
+ $this->getResult()->addValue(
+ 'error',
+ null,
+ $this->getResult()->convertStatusToArray(
$status )
+ );
$this->getResult()->addValue( null,
$this->getModuleName(), array(
'user' => $globalUser->getName(),
'locked' => $globalUser->isLocked(),
diff --git a/specials/SpecialCentralAuth.php b/specials/SpecialCentralAuth.php
index 0e68fe5..24dac25 100644
--- a/specials/SpecialCentralAuth.php
+++ b/specials/SpecialCentralAuth.php
@@ -111,13 +111,12 @@
/* deprecated */ $status->successCount
);
}
} elseif ( $this->mMethod == 'delete' && $this->mCanUnmerge ) {
- $status = $globalUser->adminDelete();
+ $status = $globalUser->adminDelete(
$this->getRequest()->getVal( 'reason' ) );
if ( !$status->isGood() ) {
$this->showStatusError( $status->getWikiText()
);
} else {
$this->showSuccess(
'centralauth-admin-delete-success', $this->mUserName );
$deleted = true;
- $this->logAction( 'delete', $this->mUserName,
$this->getRequest()->getVal( 'reason' ) );
}
} elseif ( $this->mMethod == 'set-status' && !$stateCheck ) {
$this->showError( 'centralauth-state-mismatch' );
@@ -140,17 +139,10 @@
$this->getContext()
);
- // Logging etc
+ // Tell the user what happened
if ( !$status->isGood() ) {
$this->showStatusError( $status->getWikiText()
);
} elseif ( $status->successCount > 0 ) {
- $this->logAction(
- 'setstatus',
- $globalUser->getName(),
- $reason,
- $status->success,
- $setHidden !=
CentralAuthUser::HIDDEN_NONE
- );
$this->showSuccess(
'centralauth-admin-setstatus-success', $this->mUserName );
}
} else {
@@ -682,18 +674,5 @@
);
}
return $mergeMethodDescriptions;
- }
-
- /**
- * @param $action
- * @param $target
- * @param $reason string
- * @param $params array
- * @param $suppressLog bool
- */
- function logAction( $action, $target, $reason = '', $params = array(),
$suppressLog = false ) {
- $logType = $suppressLog ? 'suppress' : 'globalauth'; // Not
centralauth because of some weird length limitiations
- $log = new LogPage( $logType );
- $log->addEntry( $action, Title::newFromText(
"User:{$target}@global" ), $reason, $params );
}
}
diff --git a/specials/SpecialMultiLock.php b/specials/SpecialMultiLock.php
index df980ab..a60c569 100644
--- a/specials/SpecialMultiLock.php
+++ b/specials/SpecialMultiLock.php
@@ -328,7 +328,6 @@
return;
}
- $sca = new SpecialCentralAuth;
$added = array();
$removed = array();
$setLocked = null;
@@ -360,13 +359,6 @@
if ( !$status->isGood() ) {
$this->showStatusError( $status->getWikiText()
);
} elseif ( $status->successCount > 0 ) {
- $sca->logAction(
- 'setstatus',
- $globalUser->getName(),
- $this->mReason,
- $status->success,
- $setHidden !=
CentralAuthUser::HIDDEN_NONE
- );
$this->showSuccess(
'centralauth-admin-setstatus-success', $globalUser->getName() );
}
}
--
To view, visit https://gerrit.wikimedia.org/r/52261
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: Idab50ad6e61f210fede7b321531fc7596fdc1b77
Gerrit-PatchSet: 3
Gerrit-Project: mediawiki/extensions/CentralAuth
Gerrit-Branch: master
Gerrit-Owner: Hoo man <[email protected]>
Gerrit-Reviewer: CSteipp <[email protected]>
Gerrit-Reviewer: Hoo man <[email protected]>
Gerrit-Reviewer: Pgehres <[email protected]>
Gerrit-Reviewer: jenkins-bot
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
