[MediaWiki-commits] [Gerrit] mediawiki/core[master]: Improve Special:BookSources validation and error messages
jenkins-bot has submitted this change and it was merged. Change subject: Improve Special:BookSources validation and error messages .. Improve Special:BookSources validation and error messages Visiting Special:BookSources/invalid would not show any error message because "invalid" would get stripped away by cleanIsbn(), and then appear as the empty string. Instead, first validate the provided ISBN (if any), and display the error message if invalid. Then show the form and book details. Tests included. Also remove an unused variable. Change-Id: I40b703eace956ebbcdc0a2c2986b2c10474dd1fd --- M includes/specials/SpecialBooksources.php M tests/phpunit/includes/specials/SpecialBooksourcesTest.php 2 files changed, 37 insertions(+), 20 deletions(-) Approvals: Florianschmidtwelzow: Looks good to me, approved jenkins-bot: Verified diff --git a/includes/specials/SpecialBooksources.php b/includes/specials/SpecialBooksources.php index 11faa28..2fef725 100644 --- a/includes/specials/SpecialBooksources.php +++ b/includes/specials/SpecialBooksources.php @@ -29,11 +29,6 @@ * @ingroup SpecialPage */ class SpecialBookSources extends SpecialPage { - /** -* ISBN passed to the page, if any -*/ - protected $isbn = ''; - public function __construct() { parent::__construct( 'Booksources' ); } @@ -49,19 +44,21 @@ $this->setHeaders(); $this->outputHeader(); - $this->isbn = self::cleanIsbn( $isbn ?: $this->getRequest()->getText( 'isbn' ) ); + // User provided ISBN + $isbn = $isbn ?: $this->getRequest()->getText( 'isbn' ); + $isbn = trim( $isbn ); - $this->buildForm(); + $this->buildForm( $isbn ); - if ( $this->isbn !== '' ) { - if ( !self::isValidISBN( $this->isbn ) ) { + if ( $isbn !== '' ) { + if ( !self::isValidISBN( $isbn ) ) { $out->wrapWikiMsg( "\n$1\n", 'booksources-invalid-isbn' ); } - $this->showList(); + $this->showList( $isbn ); } } @@ -121,20 +118,22 @@ /** * Generate a form to allow users to enter an ISBN +* +* @param string $isbn */ - private function buildForm() { + private function buildForm( $isbn ) { $formDescriptor = [ 'isbn' => [ 'type' => 'text', 'name' => 'isbn', 'label-message' => 'booksources-isbn', - 'default' => $this->isbn, + 'default' => $isbn, 'autofocus' => true, 'required' => true, ], ]; - $htmlForm = HTMLForm::factory( 'ooui', $formDescriptor, $this->getContext() ) + HTMLForm::factory( 'ooui', $formDescriptor, $this->getContext() ) ->setWrapperLegendMsg( 'booksources-search-legend' ) ->setSubmitTextMsg( 'booksources-search' ) ->setMethod( 'get' ) @@ -146,17 +145,19 @@ * Determine where to get the list of book sources from, * format and output them * +* @param string $isbn * @throws MWException * @return bool */ - private function showList() { + private function showList( $isbn ) { $out = $this->getOutput(); global $wgContLang; + $isbn = self::cleanIsbn( $isbn ); # Hook to allow extensions to insert additional HTML, # e.g. for API-interacting plugins and so on - Hooks::run( 'BookInformation', [ $this->isbn, $out ] ); + Hooks::run( 'BookInformation', [ $isbn, $out ] ); # Check for a local page such as Project:Book_sources and use that if available $page = $this->msg( 'booksources' )->inContentLanguage()->text(); @@ -169,7 +170,7 @@ // XXX: in the future, this could be stored as structured data, defining a list of book sources $text = $content->getNativeData(); - $out->addWikiText( str_replace( 'MAGICNUMBER', $this->isbn, $text ) ); + $out->addWikiText( str_replace( 'MAGICNUMBER', $isbn, $text ) ); return true; } else { @@ -182,7 +183,7 @@ $out->addHTML( '' );
[MediaWiki-commits] [Gerrit] mediawiki/core[master]: Improve Special:BookSources validation and error messages
Legoktm has uploaded a new change for review. https://gerrit.wikimedia.org/r/316047 Change subject: Improve Special:BookSources validation and error messages .. Improve Special:BookSources validation and error messages Visiting Special:BookSources/invalid would not show any error message because "invalid" would get stripped away by cleanIsbn(), and then appear as the empty string. Instead, first validate the provided ISBN (if any), and display the error message if invalid. Then show the form and book details. Tests included. Also remove an unused variable. Change-Id: I40b703eace956ebbcdc0a2c2986b2c10474dd1fd --- M includes/specials/SpecialBooksources.php M tests/phpunit/includes/specials/SpecialBooksourcesTest.php 2 files changed, 37 insertions(+), 20 deletions(-) git pull ssh://gerrit.wikimedia.org:29418/mediawiki/core refs/changes/47/316047/1 diff --git a/includes/specials/SpecialBooksources.php b/includes/specials/SpecialBooksources.php index 11faa28..2fef725 100644 --- a/includes/specials/SpecialBooksources.php +++ b/includes/specials/SpecialBooksources.php @@ -29,11 +29,6 @@ * @ingroup SpecialPage */ class SpecialBookSources extends SpecialPage { - /** -* ISBN passed to the page, if any -*/ - protected $isbn = ''; - public function __construct() { parent::__construct( 'Booksources' ); } @@ -49,19 +44,21 @@ $this->setHeaders(); $this->outputHeader(); - $this->isbn = self::cleanIsbn( $isbn ?: $this->getRequest()->getText( 'isbn' ) ); + // User provided ISBN + $isbn = $isbn ?: $this->getRequest()->getText( 'isbn' ); + $isbn = trim( $isbn ); - $this->buildForm(); + $this->buildForm( $isbn ); - if ( $this->isbn !== '' ) { - if ( !self::isValidISBN( $this->isbn ) ) { + if ( $isbn !== '' ) { + if ( !self::isValidISBN( $isbn ) ) { $out->wrapWikiMsg( "\n$1\n", 'booksources-invalid-isbn' ); } - $this->showList(); + $this->showList( $isbn ); } } @@ -121,20 +118,22 @@ /** * Generate a form to allow users to enter an ISBN +* +* @param string $isbn */ - private function buildForm() { + private function buildForm( $isbn ) { $formDescriptor = [ 'isbn' => [ 'type' => 'text', 'name' => 'isbn', 'label-message' => 'booksources-isbn', - 'default' => $this->isbn, + 'default' => $isbn, 'autofocus' => true, 'required' => true, ], ]; - $htmlForm = HTMLForm::factory( 'ooui', $formDescriptor, $this->getContext() ) + HTMLForm::factory( 'ooui', $formDescriptor, $this->getContext() ) ->setWrapperLegendMsg( 'booksources-search-legend' ) ->setSubmitTextMsg( 'booksources-search' ) ->setMethod( 'get' ) @@ -146,17 +145,19 @@ * Determine where to get the list of book sources from, * format and output them * +* @param string $isbn * @throws MWException * @return bool */ - private function showList() { + private function showList( $isbn ) { $out = $this->getOutput(); global $wgContLang; + $isbn = self::cleanIsbn( $isbn ); # Hook to allow extensions to insert additional HTML, # e.g. for API-interacting plugins and so on - Hooks::run( 'BookInformation', [ $this->isbn, $out ] ); + Hooks::run( 'BookInformation', [ $isbn, $out ] ); # Check for a local page such as Project:Book_sources and use that if available $page = $this->msg( 'booksources' )->inContentLanguage()->text(); @@ -169,7 +170,7 @@ // XXX: in the future, this could be stored as structured data, defining a list of book sources $text = $content->getNativeData(); - $out->addWikiText( str_replace( 'MAGICNUMBER', $this->isbn, $text ) ); + $out->addWikiText( str_replace( 'MAGICNUMBER', $isbn, $text ) ); return true; } else { @@ -182,7 +183,7 @@ $o