r87258 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r87257‎ | r87258 | r87259 >
Date:17:24, 2 May 2011
Author:kaldari
Status:ok (Comments)
Tags:
Comment:
FlickrChecker fixes, follow-up to r87002 and r87031
Modified paths:
  • /trunk/extensions/UploadWizard/UploadWizardHooks.php (modified) (history)
  • /trunk/extensions/UploadWizard/resources/mw.FlickrChecker.js (modified) (history)

Diff [purge]

Index: trunk/extensions/UploadWizard/UploadWizardHooks.php
@@ -60,6 +60,7 @@
6161 'resources/mw.ApiUploadHandler.js',
6262 'resources/mw.DestinationChecker.js',
6363 'resources/mw.UploadWizardUtil.js',
 64+ 'resources/mw.FlickrChecker.js',
6465
6566 // interface libraries
6667 'resources/mw.GroupProgressBar.js',
Index: trunk/extensions/UploadWizard/resources/mw.FlickrChecker.js
@@ -1,4 +1,4 @@
2 -( function( mw ) {
 2+( function( mw, $ ) {
33
44 mw.FlickrChecker = {
55
@@ -36,7 +36,7 @@
3737 var photoIdMatches = url.match(/flickr.com\/photos\/[^\/]+\/([0-9]+)/);
3838 if ( photoIdMatches && photoIdMatches[1] > 0 ) {
3939 var photoId = photoIdMatches[1];
40 - $.getJSON(this.apiUrl+'&method=flickr.photos.getInfo&api_key='+this.apiKey+'&photo_id='+photoId+'&format=json&jsoncallback=?',
 40+ $.getJSON( this.apiUrl + '?jsoncallback=?', { 'method': 'flickr.photos.getInfo', 'api_key': this.apiKey, 'photo_id': photoId, 'format': 'json' },
4141 function( data ) {
4242 if ( typeof data.photo != 'undefined' ) {
4343 // The returned data.photo.license is just an ID that we use to look up the license name
@@ -50,6 +50,7 @@
5151 } else {
5252 var licenseMessage = gM( 'mwe-upwiz-license-external', 'Flickr', licenseName );
5353 }
 54+ // XXX Do something with data.
5455 }
5556 }
5657 }
@@ -58,10 +59,10 @@
5960 },
6061
6162 /**
62 - * Retrieve the list of all current Flickr licenses and store it in an array (mw.FlickrChecker.licenses)
 63+ * Retrieve the list of all current Flickr licenses and store it in an array (mw.FlickrChecker.licenseList)
6364 */
6465 getLicenses: function() {
65 - $.getJSON(this.apiUrl+'&method=flickr.photos.licenses.getInfo&api_key='+this.apiKey+'&format=json&jsoncallback=?',
 66+ $.getJSON( this.apiUrl + '?jsoncallback=?', { 'method': 'flickr.photos.licenses.getInfo', 'api_key': this.apiKey, 'format': 'json' },
6667 function( data ) {
6768 if ( typeof data.licenses != 'undefined' ) {
6869 $.each( data.licenses.license, function(index, value) {

Follow-up revisions

RevisionCommit summaryAuthorDate
r87269fixing closure for jQuery, follow-up for r87258kaldari18:23, 2 May 2011
r87363removing extra ?skaldari22:32, 3 May 2011
r87366better api calls for flickr, fixing attribution license, restoring messages l...kaldari23:08, 3 May 2011

Past revisions this follows-up on

RevisionCommit summaryAuthorDate
r87002initial partially functioning version of FlickrCheckerkaldari07:00, 27 April 2011
r87031further work on FlickrChecker modulekaldari19:13, 27 April 2011

Comments

#Comment by Catrope (talk | contribs)   18:00, 2 May 2011
-( function( mw ) {
+( function( mw, $ ) {

You need to update the bottom end too (from (mediaWiki) to (mediaWiki, jQuery)). My understanding is that this actually breaks the plugin by setting $ to undefined.

OK otherwise.

#Comment by Kaldari (talk | contribs)   18:23, 2 May 2011

Oops! Should be fixed in r87269.

#Comment by NeilK (talk | contribs)   22:10, 3 May 2011

The arguments for getJSON are strange. Are you sure this works? I couldn't get it to do anything.

I'm not sure what you're trying to do with the '?jsoncallback=?', especially since you define apiUrl as already having a trailing '?' elsewhere. So doesn't that make '??jsoncallback=?' ?

If you just don't want a JSON callback, you can use nojsoncallback=1. This worked for me:

this.apiUrl = 'http://api.flickr.com/services/rest'; apiKey = 'blah blah key here'; $.getJSON( apiUrl, { nojsoncallback: 1, format : 'json', api_key: apiKey, method : 'flickr.panda.getList' } );

#Comment by Kaldari (talk | contribs)   23:10, 3 May 2011

Removed the extra question marks. The 'jsoncallback=?' was to generate a random callback function name (normal jsonp methodology). I didn't know you could suppress the callback function entirely. That's much more sensible. Implemented in r87366.

Status & tagging log