Thijs/wiki.techinc.nl
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Show warnings in HTMLForm and warnings as warnings on Login/Signup form

Browse source 
This commit changes the way how HTMLForm handles a Status object
when executed from a request. It now handles, beside the errors,
also the warnings of a Status object and prints them out, wrapped
in a warning box.

The LoginSignupPage uses this feature to show informative warnings
actually as warnings and not as more disturbing error messages.
Error messages should be reserved for errors and only for erros. An
AuthenticationProvider, which returns an UI AuthenticationResponse
can choose, if the given message is an error or a warning message.

This commit also addds a new function to Status, which allows a
developer to split the object into two new Status objects, where one only
contains the errors and the other only the warnings of the origin
Status object (splitByErrorType). StatusValue also has a new function,
splitByErrorType(), to support this.

Bug: T139179
Change-Id: I9a27911613e62b5c4cb86bea40696cb37c4f49c2
This commit is contained in:
Florian 2016-07-01 18:26:20 +02:00 committed by Bartosz Dziewoński
parent 5445611062
commit 3706dcb5c7
12 changed files with 222 additions and 30 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

21
includes/Status.php
View file

@ -105,6 +105,26 @@ class Status {
return new self( $sv );
}
/**
* Splits this Status object into two new Status objects, one which contains only
* the error messages, and one that contains the warnings, only. The returned array is
* defined as:
* array(
* 0 => object(Status) # the Status with error messages, only
* 1 => object(Status) # The Status with warning messages, only
* )
*
* @return array
*/
public function splitByErrorType() {
list( $errorsOnlyStatusValue, $warningsOnlyStatusValue ) = $this->sv->splitByErrorType();
$errorsOnlyStatus = new Status( $errorsOnlyStatusValue );
$warningsOnlyStatus = new Status( $warningsOnlyStatusValue );
$errorsOnlyStatus->cleanCallback = $warningsOnlyStatus->cleanCallback = $this->cleanCallback;
return [ $errorsOnlyStatus, $warningsOnlyStatus ];
}
/**
* Change operation result
*
@ -314,6 +334,7 @@ class Status {
/**
* Return the message for a single error.
*
* @param mixed $error With an array & two values keyed by
* 'message' and 'params', use those keys-value pairs.
* Otherwise, if its an array, just use the first value as the

11
includes/auth/AuthenticationResponse.php
View file

@ -81,6 +81,9 @@ class AuthenticationResponse {
/** @var Message|null I18n message to display in case of UI or FAIL */
public $message = null;
/** @var string Whether the $message is an error or warning message, for styling reasons */
public $messageType = 'warning';
/**
* @var string|null Local user name from authentication.
* May be null if the authentication passed but no local user is known.
@ -144,6 +147,7 @@ class AuthenticationResponse {
$ret = new AuthenticationResponse;
$ret->status = AuthenticationResponse::FAIL;
$ret->message = $msg;
$ret->messageType = 'error';
return $ret;
}
@ -172,18 +176,23 @@ class AuthenticationResponse {
/**
* @param AuthenticationRequest[] $reqs AuthenticationRequests needed to continue
* @param Message $msg
* @param string $msgtype
* @return AuthenticationResponse
* @see AuthenticationResponse::UI
*/
public static function newUI( array $reqs, Message $msg ) {
public static function newUI( array $reqs, Message $msg, $msgtype = 'warning' ) {
if ( !$reqs ) {
throw new \InvalidArgumentException( '$reqs may not be empty' );
}
if ( $msgtype !== 'warning' && $msgtype !== 'error' ) {
throw new \InvalidArgumentException( $msgtype . ' is not a valid message type.' );
}
$ret = new AuthenticationResponse;
$ret->status = AuthenticationResponse::UI;
$ret->neededRequests = $reqs;
$ret->message = $msg;
$ret->messageType = $msgtype;
return $ret;
}

6
includes/auth/ConfirmLinkSecondaryAuthenticationProvider.php
View file

@ -64,7 +64,8 @@ class ConfirmLinkSecondaryAuthenticationProvider extends AbstractSecondaryAuthen
$req = new ConfirmLinkAuthenticationRequest( $maybeLink );
return AuthenticationResponse::newUI(
[ $req ],
wfMessage( 'authprovider-confirmlink-message' )
wfMessage( 'authprovider-confirmlink-message' ),
'warning'
);
}
@ -150,7 +151,8 @@ class ConfirmLinkSecondaryAuthenticationProvider extends AbstractSecondaryAuthen
'linkOk', wfMessage( 'ok' ), wfMessage( 'authprovider-confirmlink-ok-help' )
)
],
$combinedStatus->getMessage( 'authprovider-confirmlink-failed' )
$combinedStatus->getMessage( 'authprovider-confirmlink-failed' ),
'error'
);
}
}

6
includes/auth/ResetPasswordSecondaryAuthenticationProvider.php
View file

@ -112,17 +112,17 @@ class ResetPasswordSecondaryAuthenticationProvider extends AbstractSecondaryAuth
$req = AuthenticationRequest::getRequestByClass( $reqs, get_class( $needReq ) );
if ( !$req || !array_key_exists( 'retype', $req->getFieldInfo() ) ) {
return AuthenticationResponse::newUI( $needReqs, $data->msg );
return AuthenticationResponse::newUI( $needReqs, $data->msg, 'warning' );
}
if ( $req->password !== $req->retype ) {
return AuthenticationResponse::newUI( $needReqs, new \Message( 'badretype' ) );
return AuthenticationResponse::newUI( $needReqs, new \Message( 'badretype' ), 'error' );
}
$req->username = $user->getName();
$status = $this->manager->allowsAuthenticationDataChange( $req );
if ( !$status->isGood() ) {
return AuthenticationResponse::newUI( $needReqs, $status->getMessage() );
return AuthenticationResponse::newUI( $needReqs, $status->getMessage(), 'error' );
}
$this->manager->changeAuthenticationData( $req );

46
includes/htmlform/HTMLForm.php
View file

@ -1014,7 +1014,8 @@ class HTMLForm extends ContextSource {
$this->getOutput()->addModuleStyles( 'mediawiki.htmlform.styles' );
$html = ''
. $this->getErrors( $submitResult )
. $this->getErrorsOrWarnings( $submitResult, 'error' )
. $this->getErrorsOrWarnings( $submitResult, 'warning' )
. $this->getHeaderText()
. $this->getBody()
. $this->getHiddenFields()
@ -1230,23 +1231,46 @@ class HTMLForm extends ContextSource {
*
* @param string|array|Status $errors
*
* @deprecated since 1.28, use getErrorsOrWarnings() instead
*
* @return string
*/
public function getErrors( $errors ) {
if ( $errors instanceof Status ) {
if ( $errors->isOK() ) {
$errorstr = '';
wfDeprecated( __METHOD__ );
return $this->getErrorsOrWarnings( $errors, 'error' );
}
/**
* Returns a formatted list of errors or warnings from the given elements.
*
* @param string|array|Status $elements The set of errors/warnings to process.
* @param string $elementsType Should warnings or errors be returned. This is meant
* for Status objects, all other valid types are always considered as errors.
* @return string
*/
public function getErrorsOrWarnings( $elements, $elementsType ) {
if ( !in_array( $elementsType, [ 'error', 'warning' ], true ) ) {
throw new DomainException( $elementsType . ' is not a valid type.' );
}
$elementstr = false;
if ( $elements instanceof Status ) {
list( $errorStatus, $warningStatus ) = $elements->splitByErrorType();
$status = $elementsType === 'error' ? $errorStatus : $warningStatus;
if ( $status->isGood() ) {
$elementstr = '';
} else {
$errorstr = $this->getOutput()->parse( $errors->getWikiText() );
$elementstr = $this->getOutput()->parse(
$status->getWikiText()
);
}
} elseif ( is_array( $errors ) ) {
$errorstr = $this->formatErrors( $errors );
} else {
$errorstr = $errors;
} elseif ( is_array( $elements ) && $elementsType === 'error' ) {
$elementstr = $this->formatErrors( $elements );
} elseif ( $elementsType === 'error' ) {
$elementstr = $elements;
}
return $errorstr
? Html::rawElement( 'div', [ 'class' => 'error' ], $errorstr )
return $elementstr
? Html::rawElement( 'div', [ 'class' => $elementsType ], $elementstr )
: '';
}

35
includes/htmlform/OOUIHTMLForm.php
View file

@ -26,6 +26,7 @@
*/
class OOUIHTMLForm extends HTMLForm {
private $oouiErrors;
private $oouiWarnings;
public function __construct( $descriptor, $context = null, $messagePrefix = '' ) {
parent::__construct( $descriptor, $context, $messagePrefix );
@ -185,28 +186,34 @@ class OOUIHTMLForm extends HTMLForm {
}
/**
* @param string|array|Status $err
* @param string|array|Status $elements
* @param string $elementsType
* @return string
*/
function getErrors( $err ) {
if ( !$err ) {
function getErrorsOrWarnings( $elements, $elementsType ) {
if ( !in_array( $elementsType, [ 'error', 'warning' ] ) ) {
throw new DomainException( $elementsType . ' is not a valid type.' );
}
if ( !$elements ) {
$errors = [];
} elseif ( $err instanceof Status ) {
if ( $err->isOK() ) {
} elseif ( $elements instanceof Status ) {
if ( $elements->isGood() ) {
$errors = [];
} else {
$errors = $err->getErrorsByType( 'error' );
$errors = $elements->getErrorsByType( $elementsType );
foreach ( $errors as &$error ) {
// Input: array( 'message' => 'foo', 'errors' => array( 'a', 'b', 'c' ) )
// Output: array( 'foo', 'a', 'b', 'c' )
$error = array_merge( [ $error['message'] ], $error['params'] );
}
}
} else {
$errors = $err;
} elseif ( $elementsType === 'errors' ) {
$errors = $elements;
if ( !is_array( $errors ) ) {
$errors = [ $errors ];
}
} else {
$errors = [];
}
foreach ( $errors as &$error ) {
@ -215,7 +222,11 @@ class OOUIHTMLForm extends HTMLForm {
}
// Used in getBody()
$this->oouiErrors = $errors;
if ( $elementsType === 'error' ) {
$this->oouiErrors = $errors;
} else {
$this->oouiWarnings = $errors;
}
return '';
}
@ -236,7 +247,10 @@ class OOUIHTMLForm extends HTMLForm {
if ( $this->oouiErrors ) {
$classes[] = 'mw-htmlform-ooui-header-errors';
}
if ( $this->mHeader || $this->oouiErrors ) {
if ( $this->oouiWarnings ) {
$classes[] = 'mw-htmlform-ooui-header-warnings';
}
if ( $this->mHeader || $this->oouiErrors || $this->oouiWarnings ) {
// if there's no header, don't create an (empty) LabelWidget, simply use a placeholder
if ( $this->mHeader ) {
$element = new OOUI\LabelWidget( [ 'label' => new OOUI\HtmlSnippet( $this->mHeader ) ] );
@ -249,6 +263,7 @@ class OOUIHTMLForm extends HTMLForm {
[
'align' => 'top',
'errors' => $this->oouiErrors,
'notices' => $this->oouiWarnings,
'classes' => $classes,
]
)

28
includes/libs/StatusValue.php
View file

@ -79,6 +79,34 @@ class StatusValue {
return $result;
}
/**
* Splits this StatusValue object into two new StatusValue objects, one which contains only
* the error messages, and one that contains the warnings, only. The returned array is
* defined as:
* array(
* 0 => object(StatusValue) # the StatusValue with error messages, only
* 1 => object(StatusValue) # The StatusValue with warning messages, only
* )
*
* @return array
*/
public function splitByErrorType() {
$errorsOnlyStatusValue = clone $this;
$warningsOnlyStatusValue = clone $this;
$warningsOnlyStatusValue->ok = true;
$errorsOnlyStatusValue->errors = $warningsOnlyStatusValue->errors = [];
foreach ( $this->errors as $item ) {
if ( $item['type'] === 'warning' ) {
$warningsOnlyStatusValue->errors[] = $item;
} else {
$errorsOnlyStatusValue->errors[] = $item;
}
};
return [ $errorsOnlyStatusValue, $warningsOnlyStatusValue ];
}
/**
* Returns whether the operation completed and didn't have any error or
* warnings

10
includes/specialpage/LoginSignupSpecialPage.php
View file

@ -354,7 +354,7 @@ abstract class LoginSignupSpecialPage extends AuthManagerSpecialPage {
$this->authAction = $this->isSignup() ? AuthManager::ACTION_CREATE_CONTINUE
: AuthManager::ACTION_LOGIN_CONTINUE;
$this->authRequests = $response->neededRequests;
$this->mainLoginForm( $response->neededRequests, $response->message, 'warning' );
$this->mainLoginForm( $response->neededRequests, $response->message, $response->messageType );
break;
default:
throw new LogicException( 'invalid AuthenticationResponse' );
@ -494,7 +494,13 @@ abstract class LoginSignupSpecialPage extends AuthManagerSpecialPage {
$form = $this->getAuthForm( $requests, $this->authAction, $msg, $msgtype );
$form->prepareForm();
$formHtml = $form->getHTML( $msg ? Status::newFatal( $msg ) : false );
$submitStatus = Status::newGood();
if ( $msg && $msgtype === 'warning' ) {
$submitStatus->warning( $msg );
} elseif ( $msg && $msgtype === 'error' ) {
$submitStatus->fatal( $msg );
}
$formHtml = $form->getHTML( $submitStatus );
$out->addHTML( $this->getPageHtml( $formHtml ) );
}

1
resources/src/mediawiki.less/mediawiki.ui/variables.less
View file

@ -51,6 +51,7 @@
@colorButtonTextActive: @colorGray7;
@colorDisabledText: @colorGray12;
@colorErrorText: #c00;
@colorWarningText: #705000;
// UI colors
@colorFieldBorder: @colorGray12;

9
resources/src/mediawiki.ui/components/forms.less
View file

@ -103,6 +103,7 @@
//
// Styleguide 5.2.
.error,
.warning,
.errorbox,
.warningbox,
.successbox {
@ -118,7 +119,13 @@
color: @colorErrorText;
border: 1px solid #fac5c5;
background-color: #fae3e3;
text-shadow: 0 1px #fae3e3;
}
// Colours taken from those for .warningbox in shared.css
.warning {
color: @colorWarningText;
border: 1px solid #fde29b;
background-color: #fdf1d1;
}
// This specifies styling for individual field validation error messages.

62
tests/phpunit/includes/StatusTest.php
View file

@ -645,4 +645,66 @@ class StatusTest extends MediaWikiLangTestCase {
];
}
/**
* @dataProvider provideErrorsWarningsOnly
* @covers Status::getErrorsOnlyStatus
* @covers Status::getWarningsOnlyStatus
*/
public function testGetErrorsWarningsOnlyStatus( $errorText, $warningText, $type, $errorResult,
$warningResult
) {
$status = Status::newGood();
if ( $errorText ) {
$status->fatal( $errorText );
}
if ( $warningText ) {
$status->warning( $warningText );
}
$testStatus = $status->splitByErrorType()[$type];
$this->assertEquals( $errorResult, $testStatus->getErrorsByType( 'error' ) );
$this->assertEquals( $warningResult, $testStatus->getErrorsByType( 'warning' ) );
}
public static function provideErrorsWarningsOnly() {
return [
[
'Just an error',
'Just a warning',
0,
[
0 => [
'type' => 'error',
'message' => 'Just an error',
'params' => []
],
],
[],
], [
'Just an error',
'Just a warning',
1,
[],
[
0 => [
'type' => 'warning',
'message' => 'Just a warning',
'params' => []
],
],
], [
null,
null,
1,
[],
[],
], [
null,
null,
0,
[],
[],
]
];
}
}

17
tests/phpunit/includes/auth/AuthenticationResponseTest.php
View file

@ -16,6 +16,7 @@ class AuthenticationResponseTest extends \MediaWikiTestCase {
public function testConstructors( $constructor, $args, $expect ) {
if ( is_array( $expect ) ) {
$res = new AuthenticationResponse();
$res->messageType = 'warning';
foreach ( $expect as $field => $value ) {
$res->$field = $value;
}
@ -51,6 +52,7 @@ class AuthenticationResponseTest extends \MediaWikiTestCase {
[ 'newFail', [ $msg ], [
'status' => AuthenticationResponse::FAIL,
'message' => $msg,
'messageType' => 'error',
] ],
[ 'newRestart', [ $msg ], [
@ -66,6 +68,21 @@ class AuthenticationResponseTest extends \MediaWikiTestCase {
'status' => AuthenticationResponse::UI,
'neededRequests' => [ $req ],
'message' => $msg,
'messageType' => 'warning',
] ],
[ 'newUI', [ [ $req ], $msg, 'warning' ], [
'status' => AuthenticationResponse::UI,
'neededRequests' => [ $req ],
'message' => $msg,
'messageType' => 'warning',
] ],
[ 'newUI', [ [ $req ], $msg, 'error' ], [
'status' => AuthenticationResponse::UI,
'neededRequests' => [ $req ],
'message' => $msg,
'messageType' => 'error',
] ],
[ 'newUI', [ [], $msg ],
new \InvalidArgumentException( '$reqs may not be empty' )