r73743 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r73742‎ | r73743 | r73744 >
Date:11:38, 25 September 2010
Author:reedy
Status:ok (Comments)
Tags:
Comment:
Actually do r73742
Modified paths:
  • /trunk/extensions/ArticleAssessmentPilot/api/ApiArticleAssessment.php (modified) (history)

Diff [purge]

Index: trunk/extensions/ArticleAssessmentPilot/api/ApiArticleAssessment.php
——@@ -182,10 +182,11 @@
183183
184184 foreach( $wgArticleAssessmentRatings as $rating ) {
185185 $ret["r{$rating}"] = array(
186 - ApiBase::PARAM_TYPE => 'integer',
 186+ ApiBase::PARAM_TYPE => 'limit',
187187 ApiBase::PARAM_DFLT => 0,
188188 ApiBase::PARAM_MIN => 0,
189189 ApiBase::PARAM_MAX => 5,
 190+ ApiBase::PARAM_MAX2 => 5,
190191 );
191192 }
192193 return $ret;

Follow-up revisions

RevisionCommit summaryAuthorDate
r737441.16wmf4: MFT r73743catrope11:40, 25 September 2010

Past revisions this follows-up on

RevisionCommit summaryAuthorDate
r73742Bug 25290 - ArticleAssessmentPilot: parameter validation...reedy11:30, 25 September 2010

Comments

#Comment by Gurch (talk | contribs)   11:50, 25 September 2010

Looking at the code I'm not sure the API's behavior of just warning for the integer type is intentional.

As with the 'limit' type, out-of-range values are set to the min/max... but then it returns the original values. Should this be changed?

#Comment by Reedy (talk | contribs)   11:53, 25 September 2010

That could be classed as breaking changes, though, I think it'd be semi valid to do so.

With limit, it doesn't: If user value > MAX (or MAX2 for bots), MAX (or MAX2) is returned.

For the integerness, I don't know how many people actually use the range validation. I've logged bug 25303 to make the ranges enforcable (ie if out of range, DIE DIE DIE)

#Comment by Gurch (talk | contribs)   11:56, 25 September 2010

Yeah, having the whole MAX/MAX2 thing for bot limit validation mixed up with general integer validation is a bit confusing. (And obviously having DIE DIE DIE behavior on limits wouldn't be a good idea).

#Comment by Duplicatebug (talk | contribs)   19:44, 2 April 2011

bug 25303 is fixed, you can change back to "integer", because No more than 5 (5 for bots) allowed looks not good for this param. Thanks.

#Comment by Reedy (talk | contribs)   23:05, 2 April 2011

It's been fixed for a while

#Comment by Duplicatebug (talk | contribs)   08:58, 3 April 2011

Which revision? The last change to that line is the revert r76457.

#Comment by Duplicatebug (talk | contribs)   16:35, 15 April 2011

Looks better after r85276

Status & tagging log