415: /api/json/reviewrequests/98/draft/set/<field>/ do not properly return errors

jaybuf*******@gmai***** (Google Code) (Is this you? Claim this profile.)
david
 david
July 26, 2009 
Doing a post to the web services API to set a field for a reviewrequest
with a missing value should result in a JSON response like this:

{
    &quot;stat&quot;: &quot;fail&quot;, 
    &quot;err&quot;: {
        &quot;msg&quot;: &quot;Missing value for Summary&quot;, 
        &quot;code&quot;: 301
    }
}

Instead I get a success response:
{&quot;stat&quot;: &quot;ok&quot;, &quot;invalid_summary&quot;: null, &quot;summary&quot;: &quot;&quot;}


This is confusing and inconsistent.    See below the full HTTP request and
response that demonstrates the problem





POST
http://codereview.corp.yahoo.com:8000/api/json/reviewrequests/98/draft/set/summary/
Content-Length: 6
Content-Type: application/x-www-form-urlencoded

value=



HTTP/1.0 200 OK
Cache-Control: max-age=0
Date: Tue, 26 Feb 2008 22:21:42 GMT
ETag: 4d7d59f3dd2e75459df8f255fa8af87f
Server: WSGIServer/0.1 Python/2.4.4
Vary: Cookie, Accept-Language
Content-Language: en-us
Content-Type: text/plain
Expires: Tue, 26 Feb 2008 22:21:42 GMT
Last-Modified: Tue, 26 Feb 2008 22:21:42 GMT
Client-Date: Tue, 26 Feb 2008 10:58:48 GMT
Client-Peer: 10.72.88.210:8000
Client-Response-Num: 1

{&quot;stat&quot;: &quot;ok&quot;, &quot;invalid_summary&quot;: null, &quot;summary&quot;: &quot;&quot;}
chipx86
#1 chipx86 
Yeah, we should fix this.

The reason it's giving you that response is that we basically reuse the code for
setting multiple fields at once. In that command, we allow the settings of fields but
return a list of invalid fields that were specified, rather than discarding
everything. We might want to rethink that, but we should definitely rethink this
command's error output.
  • +Milestone-Release1.0
    +Component-API
david
#2 david
  • +Started
  • +david
david
#3 david 
Fixed in SVN, r1218.
  • -Started
    +Fixed
david
#4 david 
Had to reopen this because the fix caused bug 443.  We'll have to make this a bit
smarter about coping with fields that are allowed to be blank.
  • -Fixed
    +New
  • -Milestone-Release1.0
david
#5 david
  • -New
    +Fixed