[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 @@
