r91670 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r91669‎ | r91670 | r91671 >
Date:19:25, 7 July 2011
Author:kaldari
Status:ok
Tags:
Comment:
Beginning to add logging for campaign setting changes. Better logic for handling different requests - fixes bug where settings could not be changed at the same time as deleting a campaign
Modified paths:
  • /trunk/extensions/CentralNotice/CentralNotice.db.php (modified) (history)
  • /trunk/extensions/CentralNotice/special/SpecialCentralNotice.php (modified) (history)

Diff [purge]

Index: trunk/extensions/CentralNotice/special/SpecialCentralNotice.php
——@@ -57,85 +57,9 @@
5858 // Check authentication token
5959 if ( $wgUser->matchEditToken( $wgRequest->getVal( 'authtoken' ) ) ) {
6060
61 - // Handle removing campaigns
62 - $toRemove = $wgRequest->getArray( 'removeNotices' );
63 - if ( $toRemove ) {
64 - // Remove campaigns in list
65 - foreach ( $toRemove as $notice ) {
66 - $this->removeNotice( $notice );
67 - }
68 -
69 - // Skip subsequent form handling and show list of campaigns
70 - $this->listNotices();
71 - $wgOut->addHTML( Xml::closeElement( 'div' ) );
72 - return;
73 - }
74 -
75 - // Handle locking/unlocking campaigns
76 - $lockedNotices = $wgRequest->getArray( 'locked' );
77 - if ( $lockedNotices ) {
78 - // Build list of campaigns to lock
79 - $unlockedNotices = array_diff( $this->getAllCampaignNames(), $lockedNotices );
80 -
81 - // Set locked/unlocked flag accordingly
82 - foreach ( $lockedNotices as $notice ) {
83 - $this->updateLock( $notice, '1' );
84 - }
85 - foreach ( $unlockedNotices as $notice ) {
86 - $this->updateLock( $notice, '0' );
87 - }
88 - // Handle updates if no post content came through (all checkboxes unchecked)
89 - } elseif ( $method !== 'addNotice' ) {
90 - $allNotices = $this->getAllCampaignNames();
91 - foreach ( $allNotices as $notice ) {
92 - $this->updateLock( $notice, '0' );
93 - }
94 - }
95 -
96 - // Handle enabling/disabling campaigns
97 - $enabledNotices = $wgRequest->getArray( 'enabled' );
98 - if ( $enabledNotices ) {
99 - // Build list of campaigns to disable
100 - $disabledNotices = array_diff( $this->getAllCampaignNames(), $enabledNotices );
101 -
102 - // Set enabled/disabled flag accordingly
103 - foreach ( $enabledNotices as $notice ) {
104 - $this->updateEnabled( $notice, '1' );
105 - }
106 - foreach ( $disabledNotices as $notice ) {
107 - $this->updateEnabled( $notice, '0' );
108 - }
109 - // Handle updates if no post content came through (all checkboxes unchecked)
110 - } elseif ( $method !== 'addNotice' ) {
111 - $allNotices = $this->getAllCampaignNames();
112 - foreach ( $allNotices as $notice ) {
113 - $this->updateEnabled( $notice, '0' );
114 - }
115 - }
116 -
117 - // Handle setting preferred campaigns
118 - $preferredNotices = $wgRequest->getArray( 'preferred' );
119 - if ( $preferredNotices ) {
120 - // Build list of campaigns to unset
121 - $unsetNotices = array_diff( $this->getAllCampaignNames(), $preferredNotices );
122 -
123 - // Set flag accordingly
124 - foreach ( $preferredNotices as $notice ) {
125 - $this->updatePreferred( $notice, '1' );
126 - }
127 - foreach ( $unsetNotices as $notice ) {
128 - $this->updatePreferred( $notice, '0' );
129 - }
130 - // Handle updates if no post content came through (all checkboxes unchecked)
131 - } elseif ( $method !== 'addNotice' ) {
132 - $allNotices = $this->getAllCampaignNames();
133 - foreach ( $allNotices as $notice ) {
134 - $this->updatePreferred( $notice, '0' );
135 - }
136 - }
137 -
138 - // Handle adding of campaign
 61+ // Handle adding a campaign
13962 if ( $method == 'addNotice' ) {
 63+
14064 $noticeName = $wgRequest->getVal( 'noticeName' );
14165 $start = $wgRequest->getArray( 'start' );
14266 $projects = $wgRequest->getArray( 'projects' );
——@@ -148,6 +72,94 @@
14973 $this->addNotice( $noticeName, '0', $start, $projects,
15074 $project_languages, $geotargeted, $geo_countries );
15175 }
 76+
 77+ // Handle changing settings to existing campaigns
 78+ } else {
 79+
 80+ // Handle removing campaigns
 81+ $toRemove = $wgRequest->getArray( 'removeNotices' );
 82+ if ( $toRemove ) {
 83+ // Remove campaigns in list
 84+ foreach ( $toRemove as $notice ) {
 85+ $this->removeNotice( $notice );
 86+ }
 87+ }
 88+
 89+ // Get all the initial campaign settings for logging
 90+ $allCampaignNames = $this->getAllCampaignNames();
 91+ $allInitialCampaignSettings = array();
 92+ foreach ( $allCampaignNames as $campaignName ) {
 93+ $allInitialCampaignSettings[$campaignName] = CentralNoticeDB::getCampaignSettings( $campaignName, false );
 94+ }
 95+
 96+ // Handle locking/unlocking campaigns
 97+ $lockedNotices = $wgRequest->getArray( 'locked' );
 98+ if ( $lockedNotices ) {
 99+ // Build list of campaigns to lock
 100+ $unlockedNotices = array_diff( $this->getAllCampaignNames(), $lockedNotices );
 101+
 102+ // Set locked/unlocked flag accordingly
 103+ foreach ( $lockedNotices as $notice ) {
 104+ $this->updateLock( $notice, '1' );
 105+ }
 106+ foreach ( $unlockedNotices as $notice ) {
 107+ $this->updateLock( $notice, '0' );
 108+ }
 109+ // Handle updates if no post content came through (all checkboxes unchecked)
 110+ } else {
 111+ $allNotices = $this->getAllCampaignNames();
 112+ foreach ( $allNotices as $notice ) {
 113+ $this->updateLock( $notice, '0' );
 114+ }
 115+ }
 116+
 117+ // Handle enabling/disabling campaigns
 118+ $enabledNotices = $wgRequest->getArray( 'enabled' );
 119+ if ( $enabledNotices ) {
 120+ // Build list of campaigns to disable
 121+ $disabledNotices = array_diff( $this->getAllCampaignNames(), $enabledNotices );
 122+
 123+ // Set enabled/disabled flag accordingly
 124+ foreach ( $enabledNotices as $notice ) {
 125+ $this->updateEnabled( $notice, '1' );
 126+ }
 127+ foreach ( $disabledNotices as $notice ) {
 128+ $this->updateEnabled( $notice, '0' );
 129+ }
 130+ // Handle updates if no post content came through (all checkboxes unchecked)
 131+ } else {
 132+ $allNotices = $this->getAllCampaignNames();
 133+ foreach ( $allNotices as $notice ) {
 134+ $this->updateEnabled( $notice, '0' );
 135+ }
 136+ }
 137+
 138+ // Handle setting preferred campaigns
 139+ $preferredNotices = $wgRequest->getArray( 'preferred' );
 140+ if ( $preferredNotices ) {
 141+ // Build list of campaigns to unset
 142+ $unsetNotices = array_diff( $this->getAllCampaignNames(), $preferredNotices );
 143+
 144+ // Set flag accordingly
 145+ foreach ( $preferredNotices as $notice ) {
 146+ $this->updatePreferred( $notice, '1' );
 147+ }
 148+ foreach ( $unsetNotices as $notice ) {
 149+ $this->updatePreferred( $notice, '0' );
 150+ }
 151+ // Handle updates if no post content came through (all checkboxes unchecked)
 152+ } else {
 153+ $allNotices = $this->getAllCampaignNames();
 154+ foreach ( $allNotices as $notice ) {
 155+ $this->updatePreferred( $notice, '0' );
 156+ }
 157+ }
 158+
 159+ // Get all the final campaign settings for logging
 160+ $allFinalCampaignSettings = array();
 161+ foreach ( $allCampaignNames as $campaignName ) {
 162+ $allFinalCampaignSettings[$campaignName] = CentralNoticeDB::getCampaignSettings( $campaignName, false );
 163+ }
152164 }
153165
154166 // If there were no errors, reload the page to prevent duplicate form submission
Index: trunk/extensions/CentralNotice/CentralNotice.db.php
——@@ -121,10 +121,11 @@
122122
123123 /*
124124 * Return settings for a campaign
125 - * @param $campaignName The name of the campaign
 125+ * @param $campaignName string: The name of the campaign
 126+ * @param $detailed boolean: Whether or not to include targeting and banner assignment info
126127 * @return an array of settings
127128 */
128 - static function getCampaignSettings( $campaignName ) {
 129+ static function getCampaignSettings( $campaignName, $detailed = true ) {
129130 global $wgCentralDBname;
130131
131132 $dbr = wfGetDB( DB_SLAVE, array(), $wgCentralDBname );
——@@ -158,24 +159,26 @@
159160 );
160161 }
161162
162 - $projects = CentralNotice::getNoticeProjects( $campaignName );
163 - $languages = CentralNotice::getNoticeLanguages( $campaignName );
164 - $geo_countries = CentralNotice::getNoticeCountries( $campaignName );
165 - $campaign['projects'] = implode( ", ", $projects );
166 - $campaign['languages'] = implode( ", ", $languages );
167 - $campaign['countries'] = implode( ", ", $geo_countries );
168 -
169 - $banners = CentralNoticeDB::getCampaignBanners( $row->not_id );
170 - // Throw out the stuff we don't need for campaign logging
171 - foreach ( $banners as $key => $row ) {
172 - unset( $banners[$key]['display_anon'] );
173 - unset( $banners[$key]['display_account'] );
174 - unset( $banners[$key]['fundraising'] );
175 - unset( $banners[$key]['landing_pages'] );
176 - unset( $banners[$key]['campaign'] );
 163+ if ( $detailed ) {
 164+ $projects = CentralNotice::getNoticeProjects( $campaignName );
 165+ $languages = CentralNotice::getNoticeLanguages( $campaignName );
 166+ $geo_countries = CentralNotice::getNoticeCountries( $campaignName );
 167+ $campaign['projects'] = implode( ", ", $projects );
 168+ $campaign['languages'] = implode( ", ", $languages );
 169+ $campaign['countries'] = implode( ", ", $geo_countries );
 170+
 171+ $banners = CentralNoticeDB::getCampaignBanners( $row->not_id );
 172+ // Throw out the stuff we don't need for campaign logging
 173+ foreach ( $banners as $key => $row ) {
 174+ unset( $banners[$key]['display_anon'] );
 175+ unset( $banners[$key]['display_account'] );
 176+ unset( $banners[$key]['fundraising'] );
 177+ unset( $banners[$key]['landing_pages'] );
 178+ unset( $banners[$key]['campaign'] );
 179+ }
 180+ // Encode into a JSON string for storage
 181+ $campaign['banners'] = FormatJson::encode( $banners );
177182 }
178 - // Encode into a JSON string for storage
179 - $campaign['banners'] = FormatJson::encode( $banners );
180183
181184 return $campaign;
182185 }

Status & tagging log