jenkins-bot has submitted this change and it was merged.
Change subject: Refactor PhotoApi tests and error handling
......................................................................
Refactor PhotoApi tests and error handling
Prefix all the errors with the operation performed when error occurred
(upload, edit) and kind of error (warning, error, unknown). Use "/" as a
delimiter. This can be later used to easily query for statistics using
MySQL's SUBSTRING_INDEX() function, e.g.:
select
substring_index(event_errorText, '/', 2) as error,
count(*) as count
from MobileWebUploads_5383883
where timestamp > '20140213000000'
group by error
order by count desc;
will show something like:
+-------------------------------------------+-------+
| error | count |
+-------------------------------------------+-------+
| upload/error | 25 |
| upload/warning | 54 |
| edit/error | 113 |
+-------------------------------------------+-------+
Also:
* Generalize error handling when warnings are returned by API.
* Remove $.toJSON() from warnings, there is no trace of those cases ever
happening in EL logs.
Change-Id: I9fa19a03506e70a567e01c0e8d29f7c5d6f62107
---
M javascripts/modules/uploads/PhotoApi.js
M tests/javascripts/modules/uploads/test_PhotoApi.js
2 files changed, 50 insertions(+), 74 deletions(-)
Approvals:
Kaldari: Looks good to me, approved
jenkins-bot: Verified
diff --git a/javascripts/modules/uploads/PhotoApi.js
b/javascripts/modules/uploads/PhotoApi.js
index 269f696..8ecb6f1 100644
--- a/javascripts/modules/uploads/PhotoApi.js
+++ b/javascripts/modules/uploads/PhotoApi.js
@@ -96,22 +96,15 @@
// FIXME: See UploadBase::checkWarnings - why these are not
errors only the MediaWiki Gods know See Bug 48261
_handleWarnings: function( result, warnings ) {
- var errorMsg = 'Missing filename: ', humanErrorMsg;
+ var errorMsg = 'upload/warning/', humanErrorMsg;
+
+ warnings = $.map( warnings, function( value, code ) {
+ return code + '/' + value;
+ } );
+ errorMsg += warnings[0] || 'unknown';
+
if ( warnings.exists ) {
- errorMsg += 'Filename exists';
humanErrorMsg = mw.msg(
'mobile-frontend-photo-upload-error-filename' );
- } else if ( warnings.badfilename ) {
- errorMsg = 'Bad filename: [' +
warnings.badfilename + ']';
- } else if ( warnings.emptyfile ) {
- errorMsg += 'Empty file';
- } else if ( warnings['filetype-unwanted-type'] ) {
- errorMsg += 'Bad filetype';
- } else if ( warnings['duplicate-archive'] ) {
- errorMsg += 'Duplicate archive';
- } else if ( warnings['large-file'] ) {
- errorMsg += 'Large file';
- } else {
- errorMsg += 'Unknown warning ' + $.toJSON(
warnings );
}
return result.reject( errorMsg, humanErrorMsg );
@@ -164,15 +157,15 @@
} ).done( function( data ) {
var descriptionUrl = '',
warnings = data.upload ?
data.upload.warnings : false,
- err = '';
+ err = 'upload/error';
if ( !data || !data.upload ) {
// error uploading image
if ( data.error ) {
if ( data.error.code ) {
- err +=
data.error.code;
+ err += '/' +
data.error.code;
}
if ( data.error.details
&& data.error.details[0] ) {
- err += ' ' +
data.error.details[0];
+ err += '/' +
data.error.details[0];
}
}
result.reject( err );
@@ -185,7 +178,7 @@
} else if ( warnings ) {
return
self._handleWarnings( result, warnings );
} else {
- return result.reject(
'Missing filename: ' + $.toJSON( data.upload ) );
+ return result.reject(
'upload/unknown/missing-filename' );
}
}
// FIXME: API doesn't return this
information on duplicate images...
@@ -201,20 +194,20 @@
result.resolve(
options.fileName, descriptionUrl );
} else if ( data &&
data.error ) {
// Edit API
error
- result.reject(
data.error.code );
+ result.reject(
'edit/error/' + data.error.code );
} else if ( data &&
data.edit && data.edit.captcha ) {
// CAPTCHAs
- result.reject(
'captcha' );
+ result.reject(
'edit/captcha' );
} else if ( data &&
data.edit && data.edit.code ) {
code =
data.edit.code;
// AbuseFilter
if (
/^abusefilter/.test( code ) ) {
-
result.reject( 'abusefilter' );
+
result.reject( 'edit/abusefilter' );
} else {
-
result.reject( code );
+
result.reject( 'edit/unknown/' + code );
}
} else {
- result.reject(
'unknown error' );
+ result.reject(
'edit/unknown' );
}
} );
} else {
diff --git a/tests/javascripts/modules/uploads/test_PhotoApi.js
b/tests/javascripts/modules/uploads/test_PhotoApi.js
index 622ebc3..3f5ab7c 100644
--- a/tests/javascripts/modules/uploads/test_PhotoApi.js
+++ b/tests/javascripts/modules/uploads/test_PhotoApi.js
@@ -1,77 +1,60 @@
( function ( $, M ) {
var photo = M.require( 'modules/uploads/_photo' ),
- PhotoApi = M.require( 'modules/uploads/PhotoApi' );
+ PhotoApi = M.require( 'modules/uploads/PhotoApi' ),
+ api;
- QUnit.module( 'MobileFrontend photo', {
+ QUnit.module( 'MobileFrontend modules/uploads/PhotoApi', {
setup: function() {
- var resp =
{"upload":{"result":"Warning","warnings":{"badfilename":"::.JPG"},"filekey":"1s.1.jpg","sessionkey":"z1.jpg"}},
- resp2 = {"warnings":{"main":{"*":"Unrecognized
parameters: 'useformat',
'r'"}},"upload":{"result":"Success","filename":"Tulip_test_2013-05-13_09-45.jpg","imageinfo":{"timestamp":"2013-05-13T16:45:53Z","user":"Jdlrobson","userid":825,"size":182912,"width":960,"height":578,"parsedcomment":"Added
photo for use on page","comment":"Added photo for use on
page","url":"http://upload.beta.wmflabs.org/wikipedia/en/b/b3/Tulip_test_2013-05-13_09-45.jpg","descriptionurl":"http://en.wikipedia.beta.wmflabs.org/wiki/File:Tulip_test_2013-05-13_09-45.jpg","sha1":"7e56537b1929d7d4d211bded2d46ba01ddbbe30f","metadata":[{"name":"JPEGFileComment","value":[{"name":0,"value":"*"}]},{"name":"MEDIAWIKI_EXIF_VERSION","value":2}],"mime":"image/jpeg","mediatype":"BITMAP","bitdepth":8}}};
+ var resp = {"warnings":{"main":{"*":"Unrecognized
parameters: 'useformat',
'r'"}},"upload":{"result":"Success","filename":"Tulip_test_2013-05-13_09-45.jpg","imageinfo":{"timestamp":"2013-05-13T16:45:53Z","user":"Jdlrobson","userid":825,"size":182912,"width":960,"height":578,"parsedcomment":"Added
photo for use on page","comment":"Added photo for use on
page","url":"http://upload.beta.wmflabs.org/wikipedia/en/b/b3/Tulip_test_2013-05-13_09-45.jpg","descriptionurl":"http://en.wikipedia.beta.wmflabs.org/wiki/File:Tulip_test_2013-05-13_09-45.jpg","sha1":"7e56537b1929d7d4d211bded2d46ba01ddbbe30f","metadata":[{"name":"JPEGFileComment","value":[{"name":0,"value":"*"}]},{"name":"MEDIAWIKI_EXIF_VERSION","value":2}],"mime":"image/jpeg","mediatype":"BITMAP","bitdepth":8}}};
- this.api = new PhotoApi();
- this.api2 = new PhotoApi();
- this.sandbox.stub( this.api, 'getToken' ).returns(
$.Deferred().resolve( 'foo' ) );
- this.sandbox.stub( this.api, 'post' ).returns(
$.Deferred().resolve( resp ) );
- this.sandbox.stub( this.api2, 'getToken' ).returns(
$.Deferred().resolve( 'foo' ) );
- this.sandbox.stub( this.api2, 'post' ).returns(
$.Deferred().resolve( resp2 ) );
+ api = new PhotoApi();
+ this.sandbox.stub( api, 'getToken' ).returns(
$.Deferred().resolve( 'foo' ) );
+ this.sandbox.stub( api, 'post' ).returns(
$.Deferred().resolve( resp ) );
}
} );
- QUnit.test( 'upload with missing filename', 1, function() {
- var badResponse;
- this.api.save( {
+ QUnit.test( '#save, upload with missing filename', 1, function( assert
) {
+ var resp =
{"upload":{"result":"Warning","warnings":{"badfilename":"::.JPG"},"filekey":"1s.1.jpg","sessionkey":"z1.jpg"}},
+ spy = this.sandbox.spy();
+
+ api.post.returns( $.Deferred().resolve( resp ) );
+
+ api.save( {
insertInPage: true,
file: {
name: '::'
},
description: 'yo:: yo ::'
- } ).fail( function() {
- badResponse = true;
- } );
- strictEqual( badResponse, true, 'The request caused a bad file
name error' );
+ } ).fail( spy );
+
+ assert.ok( spy.calledWith( 'upload/warning/badfilename/::.JPG'
), 'The request caused a bad file name error' );
} );
- QUnit.test( 'successful upload', 1, function() {
- var goodResponse;
+ QUnit.test( '#save, successful upload', 1, function( assert ) {
+ var spy = this.sandbox.spy();
- this.api2.post.
+ api.post.
withArgs( sinon.match( { action: 'edit' } ) ).
returns( $.Deferred().resolve( { edit: { result:
'Success' } } ) );
- this.api2.save( {
+ api.save( {
insertInPage: true,
file: {
name: 'z.jpg'
},
description: 'hello world'
- } ).done( function() {
- goodResponse = true;
- } );
- strictEqual( goodResponse, true, 'The request succeeded and ran
done callback' );
+ } ).done( spy );
+ assert.ok( spy.calledOnce, 'The request succeeded and ran done
callback' );
} );
- QUnit.test( 'warnings (large file)', 1, function() {
- var d = $.Deferred(),
- stub = this.sandbox.stub( d, 'reject' );
- this.api2._handleWarnings( d, { 'large-file': true } );
- strictEqual( stub.calledWith( 'Missing filename: Large file' ),
true );
- } );
-
- QUnit.test( 'warnings (existing file)', 1, function() {
- var d = $.Deferred(),
- stub = this.sandbox.stub( d, 'reject' );
- this.api2._handleWarnings( d, { exists: true } );
- strictEqual( stub.calledWith( 'Missing filename: Filename
exists', mw.msg( 'mobile-frontend-photo-upload-error-filename' ) ),
- true );
- } );
-
- QUnit.test( 'error uploading, AbuseFilter', 2, function( assert ) {
+ QUnit.test( '#save, error uploading, AbuseFilter', 2, function( assert
) {
var doneSpy = this.sandbox.spy(), failSpy = this.sandbox.spy();
- this.api2.post.
+ api.post.
returns( $.Deferred().resolve(
{"error":{"code":"verification-error","info":"This file did not pass file
verification","details":["abusefilter-warning","test",1]}} ) );
- this.api2.save( {
+ api.save( {
insertInPage: true,
file: {
name: 'z.jpg'
@@ -80,17 +63,17 @@
} ).done( doneSpy ).fail( failSpy );
assert.ok( !doneSpy.called, "don't call done" );
- assert.ok( failSpy.calledWith( 'verification-error
abusefilter-warning' ), "call fail" );
+ assert.ok( failSpy.calledWith(
'upload/error/verification-error/abusefilter-warning' ), "call fail" );
} );
- QUnit.test( 'error inserting in page, captcha', 2, function( assert ) {
+ QUnit.test( '#save. error inserting in page, captcha', 2, function(
assert ) {
var doneSpy = this.sandbox.spy(), failSpy = this.sandbox.spy();
- this.api2.post.
+ api.post.
withArgs( sinon.match( { action: 'edit' } ) ).
returns( $.Deferred().resolve( { edit: { captcha: {},
result: 'Failure' } } ) );
- this.api2.save( {
+ api.save( {
insertInPage: true,
file: {
name: 'z.jpg'
@@ -99,17 +82,17 @@
} ).done( doneSpy ).fail( failSpy );
assert.ok( !doneSpy.called, "don't call done" );
- assert.ok( failSpy.calledWith( 'captcha' ), "call fail" );
+ assert.ok( failSpy.calledWith( 'edit/captcha' ), "call fail" );
} );
QUnit.test( 'error inserting in page, AbuseFilter', 2, function( assert
) {
var doneSpy = this.sandbox.spy(), failSpy = this.sandbox.spy();
- this.api2.post.
+ api.post.
withArgs( sinon.match( { action: 'edit' } ) ).
returns( $.Deferred().resolve( { edit: { code:
'abusefilter-warning', result: 'Failure' } } ) );
- this.api2.save( {
+ api.save( {
insertInPage: true,
file: {
name: 'z.jpg'
@@ -118,7 +101,7 @@
} ).done( doneSpy ).fail( failSpy );
assert.ok( !doneSpy.called, "don't call done" );
- assert.ok( failSpy.calledWith( 'abusefilter' ), "call fail" );
+ assert.ok( failSpy.calledWith( 'edit/abusefilter' ), "call
fail" );
} );
QUnit.module( 'MobileFrontend photo: filenames' );
--
To view, visit https://gerrit.wikimedia.org/r/114367
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I9fa19a03506e70a567e01c0e8d29f7c5d6f62107
Gerrit-PatchSet: 6
Gerrit-Project: mediawiki/extensions/MobileFrontend
Gerrit-Branch: master
Gerrit-Owner: JGonera <[email protected]>
Gerrit-Reviewer: JGonera <[email protected]>
Gerrit-Reviewer: Kaldari <[email protected]>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
![http://upload.beta.wmflabs.org/wikipedia/en/b/b3/Tulip_test_2013-05-13_09-45.jpg","descriptionurl":"http://en.wikipedia.beta.wmflabs.org/wiki/File:Tulip_test_2013-05-13_09-45.jpg","sha1":"7e56537b1929d7d4d211bded2d46ba01ddbbe30f","metadata":[{"name":"JPEGFileComment","value":[{"name":0,"value":"*"}]},{"name":"MEDIAWIKI_EXIF_VERSION","value":2}],"mime":"image/jpeg","mediatype":"BITMAP","bitdepth":8}}}](http://upload.beta.wmflabs.org/wikipedia/en/b/b3/Tulip_test_2013-05-13_09-45.jpg","descriptionurl":"http://en.wikipedia.beta.wmflabs.org/wiki/File:Tulip_test_2013-05-13_09-45.jpg","sha1":"7e56537b1929d7d4d211bded2d46ba01ddbbe30f","metadata":[{"name":"JPEGFileComment","value":[{"name":0,"value":"*"}]},{"name":"MEDIAWIKI_EXIF_VERSION","value":2}],"mime":"image/jpeg","mediatype":"BITMAP","bitdepth":8}}}){kind=link}