r71060 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r71059‎ | r71060 | r71061 >
Date:03:55, 14 August 2010
Author:kaldari
Status:resolved (Comments)
Tags:
Comment:
better paging solution for edit campaign interface than r71057
Modified paths:
  • /trunk/extensions/CentralNotice/TemplatePager.php (modified) (history)

Diff [purge]

Index: trunk/extensions/CentralNotice/TemplatePager.php
@@ -24,20 +24,39 @@
2525 }
2626
2727 /**
28 - * Pull all banners from the database
 28+ * Pull banners from the database
2929 */
3030 function getQueryInfo() {
31 - return array(
32 - 'tables' => 'cn_templates',
33 - 'fields' => array( 'tmp_name', 'tmp_id' ),
34 - );
 31+ // If we are calling the pager from the manage banners interface...
 32+ if ( $this->special->mName == 'NoticeTemplate' ) {
 33+ // Return all the banners in the database
 34+ return array(
 35+ 'tables' => 'cn_templates',
 36+ 'fields' => array( 'tmp_name', 'tmp_id' ),
 37+ );
 38+ // If we are calling the pager from the campaign editing interface...
 39+ } elseif ( $this->special->mName == 'CentralNotice' ) {
 40+ $notice = $this->mRequest->getVal( 'notice' );
 41+ // Return all the banners not already assigned to the current campaign
 42+ return array(
 43+ 'tables' => array( 'cn_assignments', 'cn_templates' ),
 44+ 'fields' => array( 'cn_templates.tmp_name', 'cn_templates.tmp_id' ),
 45+ 'conds' => array( 'cn_assignments.tmp_id is null' ),
 46+ 'join_conds' => array(
 47+ 'cn_assignments' => array(
 48+ 'LEFT JOIN',
 49+ "cn_assignments.tmp_id = cn_templates.tmp_id AND cn_assignments.not_id = (SELECT not_id FROM cn_notices WHERE not_name LIKE '$notice')"
 50+ )
 51+ )
 52+ );
 53+ }
3554 }
3655
3756 /**
3857 * Sort the banner list by tmp_id
3958 */
4059 function getIndexField() {
41 - return 'tmp_id';
 60+ return 'cn_templates.tmp_id';
4261 }
4362
4463 /**
@@ -45,69 +64,57 @@
4665 */
4766 function formatRow( $row ) {
4867
49 - $templatesAssigned = array();
50 - // If we are calling the pager from the campaign editing interface...
51 - if ( $this->special->mName == 'CentralNotice' ) {
52 - // Find banners already assigned to the campaign
53 - $notice = $this->mRequest->getVal( 'notice' );
54 - $templatesAssigned = $this->selectTemplatesAssigned( $notice );
55 - }
56 -
57 - // If banner is not already assigned...
58 - if ( !in_array ( $row->tmp_name, $templatesAssigned ) ) {
59 -
60 - // Begin banner row
61 - $htmlOut = Xml::openElement( 'tr' );
62 -
63 - if ( $this->editable ) {
64 - // If we are calling the pager from the manage banners interface...
65 - if ( $this->special->mName == 'NoticeTemplate' ) {
66 - // Remove box
67 - $htmlOut .= Xml::tags( 'td', array( 'valign' => 'top' ),
68 - Xml::check( 'removeTemplates[]', false,
69 - array(
70 - 'value' => $row->tmp_name,
71 - 'onchange' => $this->onRemoveChange
72 - )
 68+ // Begin banner row
 69+ $htmlOut = Xml::openElement( 'tr' );
 70+
 71+ if ( $this->editable ) {
 72+ // If we are calling the pager from the manage banners interface...
 73+ if ( $this->special->mName == 'NoticeTemplate' ) {
 74+ // Remove box
 75+ $htmlOut .= Xml::tags( 'td', array( 'valign' => 'top' ),
 76+ Xml::check( 'removeTemplates[]', false,
 77+ array(
 78+ 'value' => $row->tmp_name,
 79+ 'onchange' => $this->onRemoveChange
7380 )
74 - );
75 - // Else, if we are calling the pager from the campaign editing interface...
76 - } elseif ( $this->special->mName == 'CentralNotice' ) {
77 - // Add box
78 - $htmlOut .= Xml::tags( 'td', array( 'valign' => 'top' ),
79 - Xml::check( 'addTemplates[]', '', array ( 'value' => $row->tmp_name ) )
80 - );
81 - // Weight select
82 - $htmlOut .= Xml::tags( 'td', array( 'valign' => 'top' ),
83 - Xml::listDropDown( "weight[$row->tmp_name]",
84 - CentralNotice::dropDownList( wfMsg( 'centralnotice-weight' ), range ( 0, 100, 5 ) ) ,
85 - '',
86 - '25',
87 - '',
88 - '' )
89 - );
90 - }
 81+ )
 82+ );
 83+ // If we are calling the pager from the campaign editing interface...
 84+ } elseif ( $this->special->mName == 'CentralNotice' ) {
 85+ // Add box
 86+ $htmlOut .= Xml::tags( 'td', array( 'valign' => 'top' ),
 87+ Xml::check( 'addTemplates[]', '', array ( 'value' => $row->tmp_name ) )
 88+ );
 89+ // Weight select
 90+ $htmlOut .= Xml::tags( 'td', array( 'valign' => 'top' ),
 91+ Xml::listDropDown( "weight[$row->tmp_name]",
 92+ CentralNotice::dropDownList( wfMsg( 'centralnotice-weight' ), range ( 0, 100, 5 ) ) ,
 93+ '',
 94+ '25',
 95+ '',
 96+ '' )
 97+ );
9198 }
92 -
93 - // Link and Preview
94 - $viewPage = SpecialPage::getTitleFor( 'NoticeTemplate', 'view' );
95 - $render = new SpecialNoticeText();
96 - $render->project = 'wikipedia';
97 - $render->language = $this->mRequest->getVal( 'wpUserLanguage' );
98 - $htmlOut .= Xml::tags( 'td', array( 'valign' => 'top' ),
99 - $this->getSkin()->makeLinkObj( $this->viewPage,
100 - htmlspecialchars( $row->tmp_name ),
101 - 'template=' . urlencode( $row->tmp_name ) ) .
102 - Xml::fieldset( wfMsg( 'centralnotice-preview' ),
103 - $render->getHtmlNotice( $row->tmp_name ),
104 - array( 'class' => 'cn-bannerpreview')
105 - )
106 - );
107 -
108 - // End banner row
109 - $htmlOut .= Xml::closeElement( 'tr' );
11099 }
111100
 101+ // Link and Preview
 102+ $viewPage = SpecialPage::getTitleFor( 'NoticeTemplate', 'view' );
 103+ $render = new SpecialNoticeText();
 104+ $render->project = 'wikipedia';
 105+ $render->language = $this->mRequest->getVal( 'wpUserLanguage' );
 106+ $htmlOut .= Xml::tags( 'td', array( 'valign' => 'top' ),
 107+ $this->getSkin()->makeLinkObj( $this->viewPage,
 108+ htmlspecialchars( $row->tmp_name ),
 109+ 'template=' . urlencode( $row->tmp_name ) ) .
 110+ Xml::fieldset( wfMsg( 'centralnotice-preview' ),
 111+ $render->getHtmlNotice( $row->tmp_name ),
 112+ array( 'class' => 'cn-bannerpreview')
 113+ )
 114+ );
 115+
 116+ // End banner row
 117+ $htmlOut .= Xml::closeElement( 'tr' );
 118+
112119 return $htmlOut;
113120 }
114121
@@ -157,34 +164,4 @@
158165 }
159166 return $htmlOut;
160167 }
161 -
162 - /**
163 - * Build a list of all the banners assigned to a campaign
164 - * @return An array of banner names
165 - */
166 - function selectTemplatesAssigned ( $notice ) {
167 - $dbr = wfGetDB( DB_SLAVE );
168 - $res = $dbr->select(
169 - array(
170 - 'cn_notices',
171 - 'cn_assignments',
172 - 'cn_templates'
173 - ),
174 - array(
175 - 'cn_templates.tmp_name',
176 - ),
177 - array(
178 - 'cn_notices.not_name' => $notice,
179 - 'cn_notices.not_id = cn_assignments.not_id',
180 - 'cn_assignments.tmp_id = cn_templates.tmp_id'
181 - ),
182 - __METHOD__,
183 - array( 'ORDER BY' => 'cn_notices.not_id' )
184 - );
185 - $templateNames = array();
186 - foreach ( $res as $row ) {
187 - array_push( $templateNames, $row->tmp_name ) ;
188 - }
189 - return $templateNames;
190 - }
191168 }

Past revisions this follows-up on

RevisionCommit summaryAuthorDate
r71057implement banner pagination for edit campaign interfacekaldari23:43, 13 August 2010

Comments

#Comment by Kaldari (talk | contribs)   20:34, 16 August 2010

This revision is dependent on the Pager.php update in r71059.

#Comment by Catrope (talk | contribs)   01:26, 24 August 2010
+						"cn_assignments.tmp_id = cn_templates.tmp_id AND cn_assignments.not_id = (SELECT not_id FROM cn_notices WHERE not_name LIKE '$notice')"

You should avoid the subquery here.

#Comment by Kaldari (talk | contribs)   21:51, 24 August 2010

fixed in r71580. Thanks for the help troubleshooting it.

#Comment by Kaldari (talk | contribs)   21:52, 24 August 2010

r71580 removes the dependency on the Pager.php update in r71059.

#Comment by Kaldari (talk | contribs)   00:41, 27 August 2010

Had to re-add dependency in r71753. The new Pager code is live now, so it's a moot issue anyway.

Status & tagging log