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

diff --git a/includes/Status.php b/includes/Status.php
index e578873..a6348b7 100644 (file)
--- a/includes/Status.php
+++ b/includes/Status.php
@@ -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

diff --git a/includes/auth/AuthenticationResponse.php b/includes/auth/AuthenticationResponse.php
index 0339e45..6684fb9 100644 (file)
--- a/includes/auth/AuthenticationResponse.php
+++ b/includes/auth/AuthenticationResponse.php
@@ -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;
        }
 

diff --git a/includes/auth/ConfirmLinkSecondaryAuthenticationProvider.php b/includes/auth/ConfirmLinkSecondaryAuthenticationProvider.php
index beb11f4..7f121cd 100644 (file)
--- a/includes/auth/ConfirmLinkSecondaryAuthenticationProvider.php
+++ b/includes/auth/ConfirmLinkSecondaryAuthenticationProvider.php
@@ -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'
                );
        }
 }

diff --git a/includes/auth/ResetPasswordSecondaryAuthenticationProvider.php b/includes/auth/ResetPasswordSecondaryAuthenticationProvider.php
index dd97830..f11a12c 100644 (file)
--- a/includes/auth/ResetPasswordSecondaryAuthenticationProvider.php
+++ b/includes/auth/ResetPasswordSecondaryAuthenticationProvider.php
@@ -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 );
 

diff --git a/includes/htmlform/HTMLForm.php b/includes/htmlform/HTMLForm.php
index 3c88594..567e692 100644 (file)
--- a/includes/htmlform/HTMLForm.php
+++ b/includes/htmlform/HTMLForm.php
@@ -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 )
                        : '';
        }
 

diff --git a/includes/htmlform/OOUIHTMLForm.php b/includes/htmlform/OOUIHTMLForm.php
index 0b22727..bbd3473 100644 (file)
--- a/includes/htmlform/OOUIHTMLForm.php
+++ b/includes/htmlform/OOUIHTMLForm.php
@@ -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,
                                                ]
                                        )

diff --git a/includes/libs/StatusValue.php b/includes/libs/StatusValue.php
index baa4e22..7f41f55 100644 (file)
--- a/includes/libs/StatusValue.php
+++ b/includes/libs/StatusValue.php
@@ -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

diff --git a/includes/specialpage/LoginSignupSpecialPage.php b/includes/specialpage/LoginSignupSpecialPage.php
index 9d17e7d..2b802ba 100644 (file)
--- a/includes/specialpage/LoginSignupSpecialPage.php
+++ b/includes/specialpage/LoginSignupSpecialPage.php
@@ -359,7 +359,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' );
@@ -499,7 +499,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 ) );
        }

diff --git a/resources/src/mediawiki.less/mediawiki.ui/variables.less b/resources/src/mediawiki.less/mediawiki.ui/variables.less
index 507109a..b6f6568 100644 (file)
--- a/resources/src/mediawiki.less/mediawiki.ui/variables.less
+++ b/resources/src/mediawiki.less/mediawiki.ui/variables.less
@@ -51,6 +51,7 @@
 @colorButtonTextActive: @colorGray7;
 @colorDisabledText: @colorGray12;
 @colorErrorText: #c00;
+@colorWarningText: #705000;
 
 // UI colors
 @colorFieldBorder: @colorGray12;

diff --git a/resources/src/mediawiki.ui/components/forms.less b/resources/src/mediawiki.ui/components/forms.less
index cc96a5c..aedec5b 100644 (file)
--- a/resources/src/mediawiki.ui/components/forms.less
+++ b/resources/src/mediawiki.ui/components/forms.less
@@ -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.

diff --git a/tests/phpunit/includes/StatusTest.php b/tests/phpunit/includes/StatusTest.php
index 474a481..ebc2d10 100644 (file)
--- a/tests/phpunit/includes/StatusTest.php
+++ b/tests/phpunit/includes/StatusTest.php
@@ -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,
+                               [],
+                               [],
+                       ]
+               ];
+       }
+
 }

diff --git a/tests/phpunit/includes/auth/AuthenticationResponseTest.php b/tests/phpunit/includes/auth/AuthenticationResponseTest.php
index 477b161..194b49e 100644 (file)
--- a/tests/phpunit/includes/auth/AuthenticationResponseTest.php
+++ b/tests/phpunit/includes/auth/AuthenticationResponseTest.php
@@ -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' )