Mwalker has submitted this change and it was merged.
Change subject: (bug 45366) CentralNotice using wrong DB for logging
......................................................................
(bug 45366) CentralNotice using wrong DB for logging
CentralNotice was not correctly querying the master DB when performing
logging operations. Instead it was querying the slave which could
result in dblag related issues. Always query the master to get id.
Additionally -- this patch fixes bugs wrt translation when using the
newly refactored code.
Change-Id: I49a53304ca8352bb62a21873274fc42331a60bdd
---
M includes/Banner.php
M includes/BannerMessageGroup.php
M special/SpecialNoticeTemplate.php
3 files changed, 34 insertions(+), 38 deletions(-)
Approvals:
Mwalker: Verified; Looks good to me, approved
jenkins-bot: Checked
diff --git a/includes/Banner.php b/includes/Banner.php
index f164f91..cec738b 100644
--- a/includes/Banner.php
+++ b/includes/Banner.php
@@ -2,6 +2,7 @@
class Banner {
var $name;
+ var $id;
function __construct( $name ) {
$this->name = $name;
@@ -20,7 +21,10 @@
}
function getId() {
- return Banner::getTemplateId( $this->name );
+ if ( !$this->id ) {
+ $this->id = Banner::getTemplateId( $this->name );
+ }
+ return $this->id;
}
function getMessageField( $field_name ) {
@@ -67,7 +71,8 @@
static function removeTemplate( $name, $user ) {
global $wgNoticeUseTranslateExtension;
- $id = Banner::getTemplateId( $name );
+ $bannerObj = new Banner( $name );
+ $id = $bannerObj->getId();
$dbr = wfGetDB( DB_SLAVE );
$res = $dbr->select( 'cn_assignments', 'asn_id', array(
'tmp_id' => $id ), __METHOD__ );
@@ -76,7 +81,8 @@
return;
} else {
// Log the removal of the banner
- Banner::logBannerChange( 'removed', $name, $user );
+ // FIXME: this log line will display changes with
inverted sense
+ $bannerObj->logBannerChange( 'removed', $user );
// Delete banner record from the CentralNotice
cn_templates table
$dbw = wfGetDB( DB_MASTER );
@@ -428,7 +434,7 @@
}
static function getTemplateId( $templateName ) {
- $dbr = wfGetDB( DB_SLAVE );
+ $dbr = wfGetDB( DB_MASTER );
$templateName = htmlspecialchars( $templateName );
$res = $dbr->select(
'cn_templates',
@@ -493,7 +499,8 @@
),
__METHOD__
);
- $bannerId = $dbw->insertId();
+ $bannerObj = new Banner( $name );
+ $bannerObj->id = $dbw->insertId();
// Perhaps these should move into the db as blobs
instead of being stored as articles
$wikiPage = new WikiPage(
@@ -511,16 +518,7 @@
Banner::updateTranslationMetadata( $pageResult, $name,
$body, $priorityLangs );
// Log the creation of the banner
- $beginSettings = array();
- $endSettings = array(
- 'anon' => $displayAnon,
- 'account' => $displayAccount,
- 'fundraising' => $fundraising,
- 'autolink' => $autolink,
- 'landingpages' => $landingPages,
- 'prioritylangs' => $priorityLangs,
- );
- Banner::logBannerChange( 'created', $name, $user,
$beginSettings, $endSettings );
+ $bannerObj->logBannerChange( 'created', $user );
}
}
@@ -567,22 +565,33 @@
* Log setting changes related to a banner
*
* @param $action string: 'created', 'modified', or 'removed'
- * @param $banner banner name
* @param $user User causing the change
* @param $beginSettings array of banner settings before changes
(optional)
- * @param $endSettings array of banner settings after changes
(optional)
- * @return int
*/
- static function logBannerChange( $action, $banner, $user,
$beginSettings = array(), $endSettings = array() ) {
+ function logBannerChange( $action, $user, $beginSettings = array() ) {
+ $endSettings = array();
+ if ( $action === 'modified' ) {
+ // Only log if there are any differences in the settings
+ $endSettings = Banner::getBannerSettings(
$this->getName(), true );
+ $changed = false;
+ foreach ( $endSettings as $key => $value ) {
+ if ( $endSettings[$key] != $beginSettings[$key]
) {
+ $changed = true;
+ }
+ }
+ if ( !$changed ) {
+ return;
+ }
+ }
+
$dbw = wfGetDB( DB_MASTER );
- $bannerObj = new Banner( $banner );
$log = array(
'tmplog_timestamp' => $dbw->timestamp(),
'tmplog_user_id' => $user->getId(),
'tmplog_action' => $action,
- 'tmplog_template_id' => $bannerObj->getId(),
- 'tmplog_template_name' => $banner,
+ 'tmplog_template_id' => $this->getId(),
+ 'tmplog_template_name' => $this->getName(),
);
foreach ( $beginSettings as $key => $value ) {
@@ -601,7 +610,5 @@
}
$dbw->insert( 'cn_template_log', $log );
- $log_id = $dbw->insertId();
- return $log_id;
}
}
diff --git a/includes/BannerMessageGroup.php b/includes/BannerMessageGroup.php
index 74be4f3..6d49b20 100644
--- a/includes/BannerMessageGroup.php
+++ b/includes/BannerMessageGroup.php
@@ -44,7 +44,7 @@
)->inContentLanguage()->plain();
// Extract the list of message fields from the banner source.
- $fields = SpecialNoticeTemplate::extractMessageFields(
$bannerSource );
+ $fields = Banner::extractMessageFields( $bannerSource );
// The MediaWiki page name convention for messages is the same
as the
// convention for banners themselves, except that it doesn't
include
@@ -58,7 +58,7 @@
}
// Build the array of message definitions.
- foreach ( $fields[1] as $msgName ) {
+ foreach ( $fields as $msgName => $msgCount ) {
$defkey = $msgDefKeyPrefix . $msgName;
$msgkey = $msgKeyPrefix . $msgName;
$definitions[$msgkey] = wfMessage( $defkey
)->inContentLanguage()->plain();
diff --git a/special/SpecialNoticeTemplate.php
b/special/SpecialNoticeTemplate.php
index ea9cec9..d409e16 100644
--- a/special/SpecialNoticeTemplate.php
+++ b/special/SpecialNoticeTemplate.php
@@ -916,18 +916,7 @@
Banner::updateTranslationMetadata( $pageResult, $name,
$body, $priorityLangs );
- // If there are any difference between the old settings
and the new settings, log them.
- $finalBannerSettings = Banner::getBannerSettings( $name, true );
- $changed = false;
- foreach ( $finalBannerSettings as $key => $value ) {
- if ( $finalBannerSettings[$key] !=
$initialBannerSettings[$key] ) {
- $changed = true;
- }
- }
-
- if ( $changed ) {
- Banner::logBannerChange( 'modified', $name,
$this->getUser(), $initialBannerSettings, $finalBannerSettings );
- }
+ $bannerObj->logBannerChange( 'modified',
$this->getUser(), $initialBannerSettings );
return;
}
--
To view, visit https://gerrit.wikimedia.org/r/50768
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I49a53304ca8352bb62a21873274fc42331a60bdd
Gerrit-PatchSet: 4
Gerrit-Project: mediawiki/extensions/CentralNotice
Gerrit-Branch: master
Gerrit-Owner: Mwalker <[email protected]>
Gerrit-Reviewer: Adamw <[email protected]>
Gerrit-Reviewer: Mwalker <[email protected]>
Gerrit-Reviewer: jenkins-bot
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
