Daniel Werner has uploaded a new change for review.

  https://gerrit.wikimedia.org/r/67561


Change subject: Deprecated wb.Site functionality related to "global" site ID in 
JavaScript
......................................................................

Deprecated wb.Site functionality related to "global" site ID in JavaScript

The concept of a "global" site ID is just a confusing concept by MediaWiki, not 
required to be
represented in our data model just like that. Instead, just having a ID (getId) 
seems sufficient.
The previous wb.Site.getId got changed to return the actual ID (previously 
getGlobalSiteId)
instead of returning the Site's related language (which can still be received by
wb.Site.getLanguageCode).
All of this is necessary since we introduced the site groups which puts us into 
the situation that
we can have multiple sites with the same language code which would result into 
multiple sites which
would return the same value when calling getId() on them.

* deprecated wb.Site.getGlobalId (use wb.Site.getId instead)
* deprecated wb.getSiteByGlobalId (use wb.getSite instead)
* In PHP, the SitesModule resource loader module now adds a "id" field instead 
of a 'globalSiteId'
  field to the "wbSiteDetails" mw.config value. Also, the object's field names 
are now the ID
  instead of the language code.
* updated tests

Change-Id: I5bb664f9bbbf095db8c70424d5e617e5796d3af2
---
M lib/includes/modules/SitesModule.php
M lib/resources/wikibase.Site.js
M lib/resources/wikibase.js
M lib/resources/wikibase.ui.SiteLinksEditTool.js
M lib/tests/qunit/wikibase.Site.tests.js
M lib/tests/qunit/wikibase.tests.js
M lib/tests/qunit/wikibase.ui.SiteLinksEditTool.tests.js
7 files changed, 63 insertions(+), 56 deletions(-)


  git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/Wikibase 
refs/changes/61/67561/1

diff --git a/lib/includes/modules/SitesModule.php 
b/lib/includes/modules/SitesModule.php
index 8713806..e5b48f2 100644
--- a/lib/includes/modules/SitesModule.php
+++ b/lib/includes/modules/SitesModule.php
@@ -57,6 +57,7 @@
 
                        if ( $site->getType() === Site::TYPE_MEDIAWIKI && 
in_array( $group, $groups ) ) {
                                $languageName = Utils::fetchLanguageName( 
$site->getLanguageCode() );
+                               $globalId = $site->getGlobalId();
 
                                // Use protocol relative URIs, as it's safe to 
assume that all wikis support the same protocol
                                list( $pageUrl, $apiUrl ) = preg_replace(
@@ -72,10 +73,10 @@
                                //$localIds = $site->getLocalIds();
                                //$name = empty( $localIds['equivalent'] ) ? 
$site->getGlobalId() : $localIds['equivalent'][0];
 
-                               $sites[$site->getLanguageCode()] = array(
+                               $sites[ $globalId ] = array(
                                        'shortName' => $languageName,
                                        'name' => $languageName, // use short 
name for both, for now
-                                       'globalSiteId' => $site->getGlobalId(),
+                                       'id' => $globalId,
                                        'pageUrl' => $pageUrl,
                                        'apiUrl' => $apiUrl,
                                        'languageCode' => 
$site->getLanguageCode(),
diff --git a/lib/resources/wikibase.Site.js b/lib/resources/wikibase.Site.js
index 4d959b3..8dffcd8 100644
--- a/lib/resources/wikibase.Site.js
+++ b/lib/resources/wikibase.Site.js
@@ -24,6 +24,8 @@
 $.extend( SELF.prototype, {
        /**
         * Returns the sites id.
+        *
+        * @since 0.4 (
         */
        getId: function() {
                return this._siteDetails.id;
@@ -32,7 +34,7 @@
        /**
         * Returns the full name of the site. This will return the name in the 
users language.
         *
-        * @param string
+        * @return string
         */
        getName: function() {
                return this._siteDetails.name;
@@ -41,7 +43,7 @@
        /**
         * Returns the short name of the site. This will return the name in the 
users language.
         *
-        * @param string
+        * @return string
         */
        getShortName: function() {
                return this._siteDetails.shortName;
@@ -50,10 +52,17 @@
        /**
         * Returns the global site id of the site.
         *
-        * @param string
+        * @deprecated Use getId() instead. There is no "global" ID anymore. 
Before, the local ID
+        *             simply was the language code which is still available by 
getLanguageCode() and
+        *             the global ID was the normal ID.
+        *
+        * NOTE: The getGlobalSiteId() in the PHP class is a concept of the 
Sites within MediaWiki and
+        *       has nothing to do with the Site in our data model.
+        *
+        * @return string
         */
        getGlobalSiteId: function() {
-               return this._siteDetails.globalSiteId;
+               return this.getId();
        },
 
        /**
diff --git a/lib/resources/wikibase.js b/lib/resources/wikibase.js
index ec1164b..bddac76 100644
--- a/lib/resources/wikibase.js
+++ b/lib/resources/wikibase.js
@@ -120,7 +120,7 @@
        /**
         * Returns an array with all the known sites.
         *
-        * @return {Object} site IDs as field names, related wb.Site objects as 
value
+        * @return {Object} Global site IDs as field names, related wb.Site 
objects as value.
         */
        this.getSites = function() {
                if( this._siteList !== null ) {
@@ -129,15 +129,17 @@
                }
 
                // get all the details about all the sites:
-               var sitesDetails = mw.config.get( 'wbSiteDetails' );
+               var sitesDetails = mw.config.get( 'wbSiteDetails' ),
+                       siteDefinition,
+                       site;
+
                this._siteList = {};
 
-               for( var siteId in sitesDetails ) {
-                       if( sitesDetails.hasOwnProperty( siteId ) ) {
-                               var site = sitesDetails[ siteId ];
-                               site.id = siteId;
-                               this._siteList[ siteId ] =  new this.Site( site 
);
-                       }
+               for( var globalSiteId in sitesDetails ) {
+                       siteDefinition = sitesDetails[ globalSiteId ];
+                       site = new this.Site( siteDefinition );
+
+                       this._siteList[ globalSiteId ] = site;
                }
 
                return this._siteList;
@@ -148,7 +150,7 @@
         *
         * @since 0.4
         *
-        * @return {Object} site IDs as field names, related wb.Site objects as 
value
+        * @return {Object} Global site IDs as field names, related wb.Site 
objects as value.
         */
        this.getSitesOfGroup = function( groupId ) {
                var sitesOfGroup = {},
@@ -196,8 +198,8 @@
        };
 
        /**
-        * Returns a wikibase.Site object with details about a site by the 
sites site ID. If there is no
-        * site related to the given ID, null will be returned.
+        * Returns a wikibase.Site object with details about a site by the 
sites ID. If there is no site
+        * related to the given ID, null will be returned.
         *
         * @param int siteId
         * @return wikibase.Site|null
@@ -213,8 +215,11 @@
        };
 
        /**
-        * Returns a wikibase.Site object with details about a site by the 
sites global ID. If there is no
-        * site related to the given ID, null will be returned.
+        * Returns a wikibase.Site object with details about a site by the 
sites global ID. If there is
+        * no site related to the given ID, null will be returned.
+        *
+        * @deprecated Use getSite() instead. The meaning of the site ID has 
changed in 0.4, also see
+        *             the Site object.
         *
         * @since 0.4
         *
@@ -222,15 +227,7 @@
         * @return wikibase.Site|null
         */
        this.getSiteByGlobalId = function( globalSiteId ) {
-               var sites = this.getSites(),
-                       site;
-               for( site in sites ) {
-                       if ( sites[ site ].getGlobalSiteId() === globalSiteId ) 
{
-                               return sites[ site ];
-                       }
-               }
-
-               return null;
+               return this.getSite( globalSiteId );
        };
 
        /**
diff --git a/lib/resources/wikibase.ui.SiteLinksEditTool.js 
b/lib/resources/wikibase.ui.SiteLinksEditTool.js
index 0d86b37..2c217a7 100644
--- a/lib/resources/wikibase.ui.SiteLinksEditTool.js
+++ b/lib/resources/wikibase.ui.SiteLinksEditTool.js
@@ -83,7 +83,7 @@
 
                if ( $( 'table.wb-sitelinks thead' ).children().length > 0 ) {
                        // initially sort on the site id column
-                       this._subject.tablesorter( { sortList: [{1:'asc'}] } );
+                       //this._subject.tablesorter( { sortList: [{1:'asc'}] } 
);
                }
        },
 
diff --git a/lib/tests/qunit/wikibase.Site.tests.js 
b/lib/tests/qunit/wikibase.Site.tests.js
index ef14f3d..918a8be 100644
--- a/lib/tests/qunit/wikibase.Site.tests.js
+++ b/lib/tests/qunit/wikibase.Site.tests.js
@@ -17,8 +17,7 @@
                setup: function() {
                        this.siteDetails = {
                                apiUrl: 'http://en.wikipedia.org/w/api.php',
-                               id: 'en',
-                               globalSiteId: 'en-global',
+                               id: 'en-global',
                                name: 'English Wikipedia',
                                pageUrl: 'http://en.wikipedia.org/wiki/$1',
                                shortName: 'English',
@@ -47,8 +46,8 @@
                );
 
                assert.strictEqual(
-                       this.site.getGlobalSiteId(),
-                       this.siteDetails.globalSiteId,
+                       this.site.getGlobalSiteId(), // deprecated, equivalent 
to getId() now
+                       this.site.getId(),
                        'verified global site id'
                );
 
diff --git a/lib/tests/qunit/wikibase.tests.js 
b/lib/tests/qunit/wikibase.tests.js
index c2be0d1..b68fc8e 100644
--- a/lib/tests/qunit/wikibase.tests.js
+++ b/lib/tests/qunit/wikibase.tests.js
@@ -30,21 +30,19 @@
         * @type Object
         */
        var TEST_SITE_DEFINITIONS = {
-               nn: {
+               nnwiki: {
                        apiUrl: '//nn.wikipedia.org/w/api.php',
-                       globalSiteId: 'nnwiki',
+                       id: 'nnwiki',
                        group: TEST_SITE_GROUPS[0],
-                       id: 'nn',
                        languageCode: 'nn',
                        name: null,
                        pageUrl: '//nn.wikipedia.org/wiki/$1',
                        shortName: 'norsk nynorsk'
                },
-               foo: {
+               'foo-site': {
                        apiUrl: '//foo.site.bar/api.php',
-                       globalSiteId: 'foo-site',
+                       id: 'foo-site',
                        group: TEST_SITE_GROUPS[1],
-                       id: 'foo',
                        languageCode: 'foo',
                        name: null,
                        pageUrl: '//foo.site.bar/pages/$1',
diff --git a/lib/tests/qunit/wikibase.ui.SiteLinksEditTool.tests.js 
b/lib/tests/qunit/wikibase.ui.SiteLinksEditTool.tests.js
index be4f2a3..6a97383 100644
--- a/lib/tests/qunit/wikibase.ui.SiteLinksEditTool.tests.js
+++ b/lib/tests/qunit/wikibase.ui.SiteLinksEditTool.tests.js
@@ -15,23 +15,23 @@
 
        var config = {
                'wbSiteDetails': {
-                       en: {
+                       enwiki: {
                                apiUrl: 'http://en.wikipedia.org/w/api.php',
-                               id: 'en',
                                name: 'English Wikipedia',
                                pageUrl: 'http://en.wikipedia.org/wiki/$1',
                                shortName: 'English',
                                languageCode: 'en',
-                               globalSiteId: 'enwiki'
+                               id: 'enwiki',
+                               group: 'whatever'
                        },
-                       de: {
+                       dewiki: {
                                apiUrl: 'http://de.wikipedia.org/w/api.php',
-                               id: 'de',
                                name: 'Deutsche Wikipedia',
                                pageUrl: 'http://de.wikipedia.org/wiki/$1',
                                shortName: 'Deutsch',
                                languageCode: 'de',
-                               globalSiteId: 'dewiki'
+                               id: 'dewiki',
+                               group: 'whatever'
                        }
                }
        };
@@ -103,24 +103,27 @@
                return newValue;
        }
 
+       var enSiteLink = [ 'enwiki', 'London' ],
+               deSiteLink = [ 'dewiki', 'Berlin' ];
+
        QUnit.test( 'getUnusedAllowedSiteIds()', function( assert ) {
                var subject = this.subject;
 
                assert.deepEqual(
                        subject.getUnusedAllowedSiteIds(),
-                       [ 'en', 'de' ],
+                       [ 'enwiki', 'dewiki' ],
                        'all allowed site links are returned as unused by newly 
initialized edit tool'
                );
 
-               hackyValueInsertionAndSave( subject, [ 'en', 'London' ], 
'enwiki' );
+               hackyValueInsertionAndSave( subject, [ 'enwiki', 'London' ], 
'enwiki' );
 
                assert.deepEqual(
                        subject.getUnusedAllowedSiteIds(),
-                       [ 'de' ],
+                       [ 'dewiki' ],
                        'after site has been used, the site will not be 
returned anymore'
                );
 
-               hackyValueInsertionAndSave( subject, [ 'de', 'Berlin' ], 
'dewiki' );
+               hackyValueInsertionAndSave( subject, [ 'dewiki', 'Berlin' ], 
'dewiki' );
 
                assert.deepEqual(
                        subject.getUnusedAllowedSiteIds(),
@@ -138,19 +141,19 @@
                        'initially, no site links, so no sites represented yet'
                );
 
-               hackyValueInsertionAndSave( subject, [ 'en', 'London' ], 
'enwiki' );
+               hackyValueInsertionAndSave( subject, [ 'English (en)', 'London' 
], 'enwiki' );
 
                assert.deepEqual(
                        subject.getRepresentedSiteIds(),
-                       [ 'en' ],
+                       [ 'enwiki' ],
                        'after site has been used, the site will be returned'
                );
 
-               hackyValueInsertionAndSave( subject, [ 'de', 'Berlin' ], 
'dewiki' );
+               hackyValueInsertionAndSave( subject, [ 'dewiki', 'Berlin' ], 
'dewiki' );
 
                assert.deepEqual(
                        subject.getRepresentedSiteIds(),
-                       [ 'en', 'de' ],
+                       [ 'enwiki', 'dewiki' ],
                        'after another site link has been entered, both sites 
will be returned'
                );
        } );
@@ -163,14 +166,14 @@
                        'not full initially'
                );
 
-               hackyValueInsertionAndSave( subject, [ 'de', 'Berlin' ], 
'dewiki' );
+               hackyValueInsertionAndSave( subject, [ 'dewiki', 'Berlin' ], 
'dewiki' );
 
                assert.ok(
                        !subject.isFull(),
                        'not full after entering one site link'
                );
 
-               hackyValueInsertionAndSave( subject, [ 'en', 'London' ], 
'enwiki' );
+               hackyValueInsertionAndSave( subject, [ 'enwiki', 'London' ], 
'enwiki' );
 
                assert.ok(
                        subject.isFull(),
@@ -214,7 +217,7 @@
                assert.ok(
                        newValue.valueCompare(
                                this.subject.getValues( true )[0].getValue(),
-                               ['de', 'Berlin']
+                               ['dewiki', 'Berlin']
                        ),
                        'new value has the value set in enterNewValue( value )'
                );

-- 
To view, visit https://gerrit.wikimedia.org/r/67561
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I5bb664f9bbbf095db8c70424d5e617e5796d3af2
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/Wikibase
Gerrit-Branch: master
Gerrit-Owner: Daniel Werner <[email protected]>

_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits

Reply via email to