Ejegg has uploaded a new change for review.
https://gerrit.wikimedia.org/r/312063
Change subject: Use booleans in function signatures
......................................................................
Use booleans in function signatures
Only cast to int just before dropping stuff in the db.
TODO: cast to boolean immediately after reading from db.
Change-Id: Ided377e93e43a569496ed3840ebc255c9c09f768
---
M includes/Banner.php
M includes/Campaign.php
M special/SpecialCentralNotice.php
M special/SpecialCentralNoticeBanners.php
M tests/CentralNoticeTest.php
M tests/CentralNoticeTestFixtures.php
6 files changed, 39 insertions(+), 43 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/CentralNotice
refs/changes/63/312063/1
diff --git a/includes/Banner.php b/includes/Banner.php
index 52477b8..550a8b4 100644
--- a/includes/Banner.php
+++ b/includes/Banner.php
@@ -1309,17 +1309,18 @@
* @param $name string name of banner
* @param $body string content of banner
* @param $user User causing the change
- * @param $displayAnon integer flag for display to anonymous users
- * @param $displayAccount integer flag for display to logged in users
- * @param $fundraising integer flag for fundraising banner
(optional)
+ * @param $displayAnon boolean flag for display to anonymous users
+ * @param $displayAccount boolean flag for display to logged in users
+ * @param $fundraising boolean flag for fundraising banner
(optional)
* @param $mixins array list of mixins (optional)
* @param $priorityLangs array Array of priority languages for the
translate extension
* @param $devices array Array of device names this banner is
targeted at
+ * @param $summary string|null Optional summary of changes for
logging
*
* @return bool true or false depending on whether banner was
successfully added
*/
static function addTemplate( $name, $body, $user, $displayAnon,
- $displayAccount, $fundraising = 0,
+ $displayAccount, $fundraising = false,
$mixins = array(), $priorityLangs = array(), $devices = null,
$summary = null
) {
@@ -1339,7 +1340,7 @@
}
$banner->setAllocation( $displayAnon, $displayAccount );
- $banner->setCategory( ( $fundraising == 1 ) ? 'fundraising' :
'{{{campaign}}}' );
+ $banner->setCategory( $fundraising ? 'fundraising' :
'{{{campaign}}}' );
$banner->setDevices( $devices );
$banner->setPriorityLanguages( $priorityLangs );
$banner->setBodyContent( $body );
diff --git a/includes/Campaign.php b/includes/Campaign.php
index cac71ac..85532ad 100644
--- a/includes/Campaign.php
+++ b/includes/Campaign.php
@@ -812,11 +812,11 @@
* Add a new campaign to the database
*
* @param $noticeName string: Name of the campaign
- * @param $enabled int: Boolean setting, 0 or 1
+ * @param $enabled bool: Boolean setting, true or false
* @param $startTs string: Campaign start in UTC
* @param $projects array: Targeted project types (wikipedia,
wikibooks, etc.)
* @param $project_languages array: Targeted project languages (en, de,
etc.)
- * @param $geotargeted int: Boolean setting, 0 or 1
+ * @param $geotargeted bool: Boolean setting, true or false
* @param $geo_countries array: Targeted countries
* @param $throttle int: limit allocations, 0 - 100
* @param $priority int: priority level, LOW_PRIORITY -
EMERGENCY_PRIORITY
@@ -851,10 +851,10 @@
$dbw->insert( 'cn_notices',
array( 'not_name' => $noticeName,
- 'not_enabled' => $enabled,
+ 'not_enabled' => (int)$enabled,
'not_start' => $dbw->timestamp( $startTs ),
'not_end' => $dbw->timestamp( $endTs ),
- 'not_geo' => $geotargeted,
+ 'not_geo' => (int)$geotargeted,
'not_throttle' => $throttle,
'not_preferred' => $priority,
)
@@ -898,10 +898,10 @@
'countries' => implode( ", ", $geo_countries ),
'start' => $dbw->timestamp( $startTs ),
'end' => $dbw->timestamp( $endTs ),
- 'enabled' => $enabled,
+ 'enabled' => (int)$enabled,
'preferred' => 0,
'locked' => 0,
- 'geo' => $geotargeted,
+ 'geo' => (int)$geotargeted,
'throttle' => $throttle,
);
Campaign::logCampaignChange( 'created', $not_id, $user,
@@ -1121,7 +1121,7 @@
*
* @param $noticeName string: Name of the campaign
* @param $settingName string: Name of a boolean setting (enabled,
locked, or geo)
- * @param $settingValue int: Value to use for the setting, 0 or 1
+ * @param $settingValue bool: Value to use for the setting, true or
false
*/
static function setBooleanCampaignSetting( $noticeName, $settingName,
$settingValue ) {
if ( !Campaign::campaignExists( $noticeName ) ) {
@@ -1131,7 +1131,7 @@
$settingName = strtolower( $settingName );
$dbw = CNDatabase::getDb( DB_MASTER );
$dbw->update( 'cn_notices',
- array( 'not_' . $settingName => $settingValue ),
+ array( 'not_' . $settingName =>
(int)$settingValue ),
array( 'not_name' => $noticeName )
);
}
diff --git a/special/SpecialCentralNotice.php b/special/SpecialCentralNotice.php
index a2da1d9..3f3c89e 100644
--- a/special/SpecialCentralNotice.php
+++ b/special/SpecialCentralNotice.php
@@ -150,19 +150,19 @@
if ( isset( $campaignChanges['archived'] ) ) {
Campaign::setBooleanCampaignSetting(
$campaignName, 'archived',
- $campaignChanges['archived'] ? 1 : 0 );
+ $campaignChanges['archived'] );
}
if ( isset( $campaignChanges['locked'] ) ) {
Campaign::setBooleanCampaignSetting(
$campaignName, 'locked',
- $campaignChanges['locked'] ? 1 : 0 );
+ $campaignChanges['locked'] );
}
if ( isset( $campaignChanges['enabled'] ) ) {
Campaign::setBooleanCampaignSetting(
$campaignName, 'enabled',
- $campaignChanges['enabled'] ? 1 : 0 );
+ $campaignChanges['enabled'] );
}
if ( isset( $campaignChanges['priority'] ) ) {
@@ -432,7 +432,7 @@
if ( $noticeName == '' ) {
$this->showError( 'centralnotice-null-string' );
} else {
- $result = Campaign::addCampaign( $noticeName, '0',
$start, $projects,
+ $result = Campaign::addCampaign( $noticeName, false,
$start, $projects,
$project_languages, $geotargeted,
$geo_countries,
100, CentralNotice::NORMAL_PRIORITY,
$this->getUser(),
$this->getSummaryFromRequest( $request ) );
@@ -578,24 +578,20 @@
// Handle removing campaign
if ( $request->getVal( 'archive' ) ) {
- Campaign::setBooleanCampaignSetting(
$notice, 'archived', 1 );
+ Campaign::setBooleanCampaignSetting(
$notice, 'archived', true );
}
$initialCampaignSettings =
Campaign::getCampaignSettings( $notice );
// Handle locking/unlocking campaign
- if ( $request->getCheck( 'locked' ) ) {
- Campaign::setBooleanCampaignSetting(
$notice, 'locked', 1 );
- } else {
- Campaign::setBooleanCampaignSetting(
$notice, 'locked', 0 );
- }
+ Campaign::setBooleanCampaignSetting(
+ $notice, 'locked', $request->getCheck(
'locked' )
+ );
// Handle enabling/disabling campaign
- if ( $request->getCheck( 'enabled' ) ) {
- Campaign::setBooleanCampaignSetting(
$notice, 'enabled', 1 );
- } else {
- Campaign::setBooleanCampaignSetting(
$notice, 'enabled', 0 );
- }
+ Campaign::setBooleanCampaignSetting(
+ $notice, 'enabled', $request->getCheck(
'enabled' )
+ );
// Set campaign traffic throttle
if ( $request->getCheck( 'throttle-enabled' ) )
{
@@ -628,13 +624,13 @@
// Handle updating geotargeting
if ( $request->getCheck( 'geotargeted' ) ) {
- Campaign::setBooleanCampaignSetting(
$notice, 'geo', 1 );
+ Campaign::setBooleanCampaignSetting(
$notice, 'geo', true );
$countries = $request->getArray(
'geo_countries' );
if ( $countries ) {
Campaign::updateCountries(
$notice, $countries );
}
} else {
- Campaign::setBooleanCampaignSetting(
$notice, 'geo', 0 );
+ Campaign::setBooleanCampaignSetting(
$notice, 'geo', false );
}
// Handle updating the start and end settings
diff --git a/special/SpecialCentralNoticeBanners.php
b/special/SpecialCentralNoticeBanners.php
index f8359a9..b868194 100755
--- a/special/SpecialCentralNoticeBanners.php
+++ b/special/SpecialCentralNoticeBanners.php
@@ -264,7 +264,7 @@
false,
false,
// Default values of a
zillion parameters...
- 0, array(), array(),
null,
+ false, array(),
array(), null,
$formData['newBannerEditSummary']
);
diff --git a/tests/CentralNoticeTest.php b/tests/CentralNoticeTest.php
index b7ecc8d..5a9a759 100644
--- a/tests/CentralNoticeTest.php
+++ b/tests/CentralNoticeTest.php
@@ -27,13 +27,13 @@
parent::setUp();
self::$centralNotice = new CentralNotice;
$noticeName = 'PHPUnitTestCampaign';
- $enabled = 0;
+ $enabled = false;
$startTs = '20110718' . '235500';
$projects = array( 'wikipedia', 'wikibooks' );
$languages = array( 'en', 'de' );
- $geotargeted = 1;
+ $geotargeted = true;
$countries = array( 'US', 'AF' );
- $preferred = 1;
+ $priority = 1;
$this->fixture = new CentralNoticeTestFixtures();
$this->fixture->setupTestCaseWithDefaults(
@@ -65,17 +65,16 @@
}
Campaign::addCampaign( $noticeName, $enabled, $startTs,
$projects,
$languages, $geotargeted, $countries,
- 100, $preferred, $this->userUser );
+ 100, $priority, $this->userUser );
$this->campaignId = Campaign::getNoticeId(
'PHPUnitTestCampaign' );
self::$noticeTemplate = new SpecialNoticeTemplate;
$bannerName = 'PHPUnitTestBanner';
$body = 'testing';
- $displayAnon = 1;
- $displayAccount = 1;
- $fundraising = 1;
- $campaign_z_index = 1;
+ $displayAnon = true;
+ $displayAccount = true;
+ $fundraising = true;
$this->campaignBannersJson =
'[{"name":"PHPUnitTestBanner","weight":25,"display_anon":1,"display_account":1,"fundraising":1,"device":"desktop","campaign":"PHPUnitTestCampaign","campaign_z_index":"1","campaign_num_buckets":1,"campaign_throttle":100,"bucket":0}]';
diff --git a/tests/CentralNoticeTestFixtures.php
b/tests/CentralNoticeTestFixtures.php
index 33e4c1d..221dbe8 100644
--- a/tests/CentralNoticeTestFixtures.php
+++ b/tests/CentralNoticeTestFixtures.php
@@ -230,14 +230,14 @@
$campaign['id'] = Campaign::addCampaign(
$campaign['name'],
- (int)$campaign['enabled'],
+ $campaign['enabled'],
$campaign['startTs'],
$campaign['projects'],
$campaign['languages'],
- (int)$campaign['geotargeted'],
+ $campaign['geotargeted'],
$campaign['countries'],
$campaign['throttle'],
- (int)$campaign['preferred'],
+ $campaign['preferred'],
$this->user
);
@@ -273,7 +273,7 @@
Campaign::setBooleanCampaignSetting(
$campaign['name'],
'archived',
- (int)$campaign['archived']
+ $campaign['archived']
);
}
--
To view, visit https://gerrit.wikimedia.org/r/312063
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ided377e93e43a569496ed3840ebc255c9c09f768
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/CentralNotice
Gerrit-Branch: master
Gerrit-Owner: Ejegg <eeggles...@wikimedia.org>
_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
