r95861 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r95860‎ | r95861 | r95862 >
Date:13:21, 31 August 2011
Author:akshay
Status:ok (Comments)
Tags:
Comment:
Followup r95197, fixed code to display extension specific messages
Modified paths:
  • /trunk/extensions/SignupAPI/SignupAPI.i18n.php (modified) (history)
  • /trunk/extensions/SignupAPI/SignupAPI.php (modified) (history)
  • /trunk/extensions/SignupAPI/includes/ApiValidateSignup.php (modified) (history)
  • /trunk/extensions/SignupAPI/includes/verification.js (modified) (history)

Diff [purge]

Index: trunk/extensions/SignupAPI/SignupAPI.i18n.php
@@ -11,6 +11,8 @@
1212 $messages['en'] = array(
1313 'signupapi-desc' => 'Cleans up the [[Special:UserLogin|login page]] from signup related stuff and adds an API for signup',
1414 'signupapi-ok' => 'OK',
 15+ 'signupapi-noname' => 'No username was specified',
 16+ 'signupapi-userexists' => 'User exists',
1517 'signupapi-enterpassword' => 'You must enter a password',
1618 'signupapi-passwordtooshort' => 'Password is too short',
1719 'signupapi-weak' => 'Weak',
@@ -18,7 +20,7 @@
1921 'signupapi-strong' => 'Strong',
2022 'signupapi-badretype' => 'The passwords you entered do not match',
2123 'signupapi-passwordsmatch' => 'Passwords match',
22 -
 24+ 'signupapi-invalidemailaddress' => 'Email address in invalid',
2325 );
2426
2527 /** Belarusian (Taraškievica orthography) (Беларуская (тарашкевіца))
Index: trunk/extensions/SignupAPI/SignupAPI.php
@@ -52,13 +52,16 @@
5353 'scripts' => 'includes/verification.js',
5454 'messages' => array(
5555 'signupapi-ok',
 56+ 'signupapi-noname',
 57+ 'signupapi-userexists',
5658 'signupapi-enterpassword',
5759 'signupapi-passwordtooshort',
5860 'signupapi-weak',
5961 'signupapi-medium',
6062 'signupapi-strong',
6163 'signupapi-badretype',
62 - 'signupapi-passwordsmatch'
 64+ 'signupapi-passwordsmatch',
 65+ 'signupapi-invalidemailaddress',
6366 ),
6467 'dependencies' => array( 'jquery.ui.progressbar' ),
6568 'localBasePath' => dirname( __FILE__ ),
Index: trunk/extensions/SignupAPI/includes/ApiValidateSignup.php
@@ -25,28 +25,28 @@
2626 case "username":
2727 $mUser = User::newFromName( $params['inputVal'], 'creatable' );
2828 if ( !is_object( $mUser ) ) {
29 - $result['result'] = wfMsg( 'noname' );
 29+ $result['result'] = wfMsg( 'signupapi-noname' );
3030 $result['icon'] = 'MW-Icon-AlertMark.png';
3131 }
3232
3333 if ( 0 != $mUser->idForName() ) {
34 - $result['result'] = wfMsg( 'userexists' );
 34+ $result['result'] = wfMsg( 'signupapi-userexists' );
3535 $result['icon'] = "MW-Icon-NoMark.png";
3636 }
3737
3838 else {
39 - $result['result'] = wfMsg( 'ok' );
 39+ $result['result'] = wfMsg( 'signupapi-ok' );
4040 $result['icon'] = "MW-Icon-CheckMark.png";
4141 }
4242 break;
4343
4444 case "email" :
4545 if ( $valid = User::isValidEmailAddr( $params['inputVal'] ) ) {
46 - $result['result']= wfMsg( 'ok' );
 46+ $result['result']= wfMsg( 'signupapi-ok' );
4747 $result['icon'] = "MW-Icon-CheckMark.png";
4848 }
4949 else {
50 - $result['result']= wfMsg( 'invalidemailaddress' );
 50+ $result['result']= wfMsg( 'signupapi-invalidemailaddress' );
5151 $result['icon'] = "MW-Icon-NoMark.png";
5252 }
5353 break;
Index: trunk/extensions/SignupAPI/includes/verification.js
@@ -27,7 +27,7 @@
2828 if (pwd.value.length==0) {
2929 strength.innerHTML = mw.message( 'signupapi-enterpassword' );
3030 } else if (pwd.value.length<minlength) {
31 - strength.innerHTML = mw.message( 'passwordtooshort', minlength );
 31+ strength.innerHTML = mw.message( 'signupapi-passwordtooshort', minlength );
3232 $("#progress").progressbar({value: 10});
3333 $("div.ui-progressbar-value").css("background","red");
3434 } else if (strongRegex.test(pwd.value)) {
@@ -55,7 +55,7 @@
5656 message = mw.message( 'signupapi-passwordsmatch' );
5757 }else {
5858 image = "<img src='"+ imagePath +"MW-Icon-NoMark.png'>";
59 - message = mw.message( 'badretype' );
 59+ message = mw.message( 'signupapi-badretype' );
6060 }
6161 valresult.innerHTML = image+message;
6262 }

Past revisions this follows-up on

RevisionCommit summaryAuthorDate
r95197Followup r95185, added all the messaged used by the extensionakshay13:07, 22 August 2011

Comments

#Comment by Reedy (talk | contribs)   22:36, 22 September 2011

Adding to the latest revision for ease...

Few issues:

  • On SpecialUserSignup
    • Line 443 $tempUser is undefined
    • Line 479 $ip is undefined
    • Line 495 $valid is undefined
    • Line 516 $abortError is undefined
    • Line 931, $this->successfulLogin() is undefined
  • On ApiSignup
    • Line 56 the hook call for "UserLoginComplete" seems useless on an signup form
    • Line 59 the hook BeforeWelcomeCreation seems useless as the information isn't returned or anything else
  • On ApiValidateSignup
    • The api shouldn't in most cases be parsing messages for return in the result. Passing just a set string is enough. Same for the images being returned. It would look like in verification.js you're doing some stuff with the images there too
  • SignupAPI.hooks
    • Rather than building query strings yourself, you should look at using wfAppendQuery()

Question: Which version of MediaWiki did you develop this against?


Few bits that against trunk is pointing at deprecated code, but that is a minor issue.


Any chance you could address these issues? Thanks!

#Comment by Akshay.agarwal (talk | contribs)   06:13, 23 September 2011

Hi Reedy,

         Thanks for pointing out these issues, currently I have my exams going on & will solve this issues ASAP after exam completion
#Comment by Reedy (talk | contribs)   18:52, 28 September 2011

Also, see the CR on r98345

Status & tagging log