r100935 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r100934‎ | r100935 | r100936 >
Date:01:51, 27 October 2011
Author:khorn
Status:ok (Comments)
Tags:fundraising 
Comment:
Addresses a couple fixme-type comments from r98202.
Modified paths:
  • /trunk/extensions/DonationInterface/gateway_common/GatewayForm.php (modified) (history)
  • /trunk/extensions/DonationInterface/gateway_common/gateway.adapter.php (modified) (history)

Diff [purge]

Index: trunk/extensions/DonationInterface/gateway_common/gateway.adapter.php
——@@ -252,7 +252,7 @@
253253 }
254254
255255 function getData( $val = '' ) {
256 - if ( empty( $val ) ) {
 256+ if ( $val === '' ) {
257257 return $this->postdata;
258258 } else {
259259 if ( array_key_exists( $val, $this->postdata ) ) {
Index: trunk/extensions/DonationInterface/gateway_common/GatewayForm.php
——@@ -23,20 +23,6 @@
2424 class GatewayForm extends UnlistedSpecialPage {
2525
2626 /**
27 - * Defines the action to take on a transaction.
28 - *
29 - * Possible values include 'process', 'challenge',
30 - * 'review', 'reject'. These values can be set during
31 - * data processing validation, for instance.
32 - *
33 - * Hooks are exposed to handle the different actions.
34 - *
35 - * Defaults to 'process'.
36 - * @var string
37 - */
38 - public $action = 'process';
39 -
40 - /**
4127 * A container for the form class
4228 *
4329 * Used to loard the form object to display the CC form
——@@ -367,48 +353,48 @@
368354 $wgOut->addHTML( HTML::element( 'span', null, $results['message'] ) );
369355
370356 if ( !empty( $results['errors'] ) ) {
371 - $wgOut->addHTML( "<ul>" );
 357+ $wgOut->addHTML( HTML::openElement( 'ul' ) );
372358 foreach ( $results['errors'] as $code => $value ) {
373 - $wgOut->addHTML( HTML::element('li', null, "Error $code: $value" ) );
 359+ $wgOut->addHTML( HTML::element('li', null, "Error $code: $value" ) . HTML::closeElement( 'li' ) );
374360 }
375 - $wgOut->addHTML( "</ul>" );
 361+ $wgOut->addHTML( HTML::closeElement( 'ul' ) );
376362 }
377363
378364 if ( !empty( $results['data'] ) ) {
379 - $wgOut->addHTML( "<ul>" );
 365+ $wgOut->addHTML( HTML::openElement( 'ul' ) );
380366 foreach ( $results['data'] as $key => $value ) {
381367 if ( is_array( $value ) ) {
382 - $wgOut->addHTML( HTML::element('li', null, $key ) . '<ul>' );
 368+ $wgOut->addHTML( HTML::element('li', null, $key ) . HTML::openElement( 'ul' ) );
383369 foreach ( $value as $key2 => $val2 ) {
384 - $wgOut->addHTML( HTML::element('li', null, "$key2: $val2" ) );
 370+ $wgOut->addHTML( HTML::element('li', null, "$key2: $val2" ) . HTML::closeElement( 'li' ) );
385371 }
386 - $wgOut->addHTML( "</ul>" );
 372+ $wgOut->addHTML( HTML::closeElement( 'ul' ) . HTML::closeElement( 'li' ) );
387373 } else {
388 - $wgOut->addHTML( HTML::element('li', null, "$key: $value" ) );
 374+ $wgOut->addHTML( HTML::element('li', null, "$key: $value" ) . HTML::closeElement( 'li' ) );
389375 }
390376 }
391 - $wgOut->addHTML( "</ul>" );
 377+ $wgOut->addHTML( HTML::closeElement( 'ul' ) );
392378 } else {
393379 $wgOut->addHTML( "Empty Results" );
394380 }
395381 if ( array_key_exists( 'Donor', $_SESSION ) ) {
396 - $wgOut->addHTML( "Session Donor Vars:<ul>" );
 382+ $wgOut->addHTML( "Session Donor Vars:" . HTML::openElement( 'ul' ));
397383 foreach ( $_SESSION['Donor'] as $key => $val ) {
398 - $wgOut->addHTML( HTML::element('li', null, "$key: $val" ) );
 384+ $wgOut->addHTML( HTML::element('li', null, "$key: $val" ) . HTML::closeElement( 'li' ) );
399385 }
400 - $wgOut->addHTML( "</ul>" );
 386+ $wgOut->addHTML( HTML::closeElement( 'ul' ) );
401387 } else {
402 - $wgOut->addHTML( "No Session Donor Vars:<ul>" );
 388+ $wgOut->addHTML( "No Session Donor Vars:" );
403389 }
404390
405391 if ( is_array( $this->adapter->debugarray ) ) {
406 - $wgOut->addHTML( "Debug Array:<ul>" );
 392+ $wgOut->addHTML( "Debug Array:" . HTML::openElement( 'ul' ) );
407393 foreach ( $this->adapter->debugarray as $val ) {
408 - $wgOut->addHTML( HTML::element('li', null, $val ) );
 394+ $wgOut->addHTML( HTML::element('li', null, $val ) . HTML::closeElement( 'li' ) );
409395 }
410 - $wgOut->addHTML( "</ul>" );
 396+ $wgOut->addHTML( HTML::closeElement( 'ul' ) );
411397 } else {
412 - $wgOut->addHTML( "No Debug Array<ul>" );
 398+ $wgOut->addHTML( "No Debug Array" );
413399 }
414400 }
415401

Follow-up revisions

RevisionCommit summaryAuthorDate
r100938follow-up to r100935, the Html::element method is self-closingkaldari02:54, 27 October 2011

Past revisions this follows-up on

RevisionCommit summaryAuthorDate
r98202More refactoring work for the gateway adapters, mostly surrounding the specia...khorn00:48, 27 September 2011

Comments

#Comment by 😂 (talk | contribs)   02:15, 27 October 2011

I know this dates to before your commit, but while you're working on it...how about removing a bunch of those addHTML() function calls? You could just make an $html string that you append to, and then just $wgOut->addHTML( $html ) at the end.

#Comment by Awjrichards (talk | contribs)   02:26, 27 October 2011

Good call. I'm marking this OK for now as I'm hoping to get this stuff merged into deployment ASAP.

Status & tagging log