Daniel Werner has uploaded a new change for review.
https://gerrit.wikimedia.org/r/67394
Change subject: (bug 38201) Introduction of "allowedSites" option in
SiteLinksEditTool
......................................................................
(bug 38201) Introduction of "allowedSites" option in SiteLinksEditTool
* "allowedSites" allows to set a set of sites the edit tool should allow for
creating site links
* introduces SiteLinksEditTool.getUnusedAllowedSiteIds()
* adds some missing SiteLinksEditTool tests and does some cleanup there as well
* using the new feature, providing all SiteLinksEditTool instances with the
right set of site links
(depending on the group) in wikibase.ui.entityViewInit.js
Change-Id: I6524c4f1c97b072e08cec05c67e8b7e4b3b4b06d
---
M lib/resources/jquery.wikibase/jquery.wikibase.siteselector.js
M lib/resources/wikibase.ui.PropertyEditTool.EditableSiteLink.js
M lib/resources/wikibase.ui.PropertyEditTool.EditableValue.SiteIdInterface.js
M lib/resources/wikibase.ui.SiteLinksEditTool.js
M lib/tests/qunit/wikibase.ui.SiteLinksEditTool.tests.js
M repo/resources/wikibase.ui.entityViewInit.js
6 files changed, 215 insertions(+), 67 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/Wikibase
refs/changes/94/67394/1
diff --git a/lib/resources/jquery.wikibase/jquery.wikibase.siteselector.js
b/lib/resources/jquery.wikibase/jquery.wikibase.siteselector.js
index 119aff8..8bb7e32 100644
--- a/lib/resources/jquery.wikibase/jquery.wikibase.siteselector.js
+++ b/lib/resources/jquery.wikibase/jquery.wikibase.siteselector.js
@@ -19,7 +19,7 @@
* @example $( 'input' ).siteselector( { resultSet: < list of wikibase
Site objects > } );
* @desc Creates a simple site selector.
*
- * @option resultSet {wb.Site[]} Set of sites that shall be filtered
for any result(s).
+ * @option resultSet {wb.Site[]} Set of sites that should be available
as selectable options.
*/
$.widget( 'wikibase.siteselector', $.ui.suggester, {
/**
diff --git a/lib/resources/wikibase.ui.PropertyEditTool.EditableSiteLink.js
b/lib/resources/wikibase.ui.PropertyEditTool.EditableSiteLink.js
index d470b41..ee70ac3 100644
--- a/lib/resources/wikibase.ui.PropertyEditTool.EditableSiteLink.js
+++ b/lib/resources/wikibase.ui.PropertyEditTool.EditableSiteLink.js
@@ -258,7 +258,7 @@
preserveEmptyForm: false,
/**
- * Allows to specify an array with sites which should not be allowed to
choose
+ * Allows to specify an array with sites which should not be allowed to
be chosen
* @var wikibase.Site[]
*/
ignoredSiteLinks: null
diff --git
a/lib/resources/wikibase.ui.PropertyEditTool.EditableValue.SiteIdInterface.js
b/lib/resources/wikibase.ui.PropertyEditTool.EditableValue.SiteIdInterface.js
index 220d825..4edcb73 100644
---
a/lib/resources/wikibase.ui.PropertyEditTool.EditableValue.SiteIdInterface.js
+++
b/lib/resources/wikibase.ui.PropertyEditTool.EditableValue.SiteIdInterface.js
@@ -109,7 +109,7 @@
}
}
- // find out which site ids should be selectable and add them as
auto selct choice
+ // find out which site ids should be selectable and add them as
auto select choice
this._currentResults = [];
for ( var siteId in wb.getSites() ) {
if( $.inArray( siteId, ignoredSites ) === -1 ) {
diff --git a/lib/resources/wikibase.ui.SiteLinksEditTool.js
b/lib/resources/wikibase.ui.SiteLinksEditTool.js
index d6ba1bc..0d86b37 100644
--- a/lib/resources/wikibase.ui.SiteLinksEditTool.js
+++ b/lib/resources/wikibase.ui.SiteLinksEditTool.js
@@ -41,7 +41,7 @@
var self = this;
// setting default options
- options = $.extend( {}, PARENT.prototype._options, {
+ this._options = $.extend( {}, PARENT.prototype._options, {
/**
* @see
wikibase.ui.PropertyEditTool.allowsMultipleValues
*/
@@ -50,10 +50,29 @@
/**
* @see wikibase.ui.PropertyEditTool.fullListMessage
*/
- fullListMessage: mw.msg(
'wikibase-sitelinksedittool-full' )
+ fullListMessage: mw.msg(
'wikibase-sitelinksedittool-full' ),
+
+ /**
+ * Should contain all the sites that can be used in
creating site links
+ * @type wb.Site[]|Object
+ */
+ allowedSites: []
} );
PARENT.prototype._init.call( this, subject, options );
+
+ // TODO: Just introduce a SiteList prototype which will make
all of the (even incomplete)
+ // validation here pointless, allowing to get rid of some
complexity here.
+ var allowedSites = this.getOption( 'allowedSites' );
+
+ if( !$.isArray( allowedSites ) && !$.isPlainObject(
allowedSites ) ) {
+ throw new Error( '"allowedSites" option has to be an
array or plain object' );
+ }
+ $.each( allowedSites, function( i, site ) {
+ if( !( site instanceof wb.Site ) ) {
+ throw new Error( '"allowedSites" option has to
be a set of wb.Site objects' );
+ }
+ } );
this._fixateTableLayout();
@@ -66,7 +85,6 @@
// initially sort on the site id column
this._subject.tablesorter( { sortList: [{1:'asc'}] } );
}
-
},
/**
@@ -95,9 +113,6 @@
*/
_initEditToolForValues: function() {
PARENT.prototype._initEditToolForValues.call( this );
-
- // make sure selecting a language in EditableSiteLink only
offers languages not yet chosen
- this.getEditableValuePrototype().prototype.ignoredSiteLinks =
this.getRepresentedSiteIds();
},
/**
@@ -179,26 +194,7 @@
* @return {Boolean}
*/
isFull: function() {
- var allSites = wb.getSites(),
- usedSiteIds = this.getRepresentedSiteIds();
-
- // Check each site if it is represented since there may be
invalid sites in the DB.
- var isFull = true;
- $.each( allSites, function( siteId, site ) {
- var found = false;
- $.each( usedSiteIds, function( i, usedSiteId ) {
- if ( usedSiteId === site.getId() ) {
- found = true;
- return false; // break
- }
- } );
- if ( !found ) {
- isFull = false;
- return false; // break
- }
- } );
-
- return isFull;
+ return this.getUnusedAllowedSiteIds().length === 0;
},
/**
@@ -245,7 +241,23 @@
EnhancedProto = function() { BaseProto.apply( this,
arguments ); };
EnhancedProto.prototype = new BaseProto();
- EnhancedProto.prototype.ignoredSiteLinks =
this.getRepresentedSiteIds();
+
+ // Make sure selecting a language in EditableSiteLink only
offers sites not yet chosen
+ // and sites this edit tool is actually responsible for (e.g.
because they are in a certain
+ // group).
+ var ignoredSiteIds = [],
+ unusedAllowedSiteIds = this.getUnusedAllowedSiteIds();
+
+ // TODO: very ugly to access global set of all sites here. Also
kind of ugly to do this by
+ // abusing the prototype field of the EditableValue instances
here. Instead, the
+ // "ignoredSiteLinks" thing (which is even named wrong) should
go away, instead we should
+ // have "allowedSites" options all the way to the
SiteIdInterface.
+ $.each( wb.getSites(), function( siteId, site ) {
+ if( $.inArray( siteId, unusedAllowedSiteIds ) === -1 ) {
+ ignoredSiteIds.push( siteId );
+ }
+ } );
+ EnhancedProto.prototype.ignoredSiteLinks = ignoredSiteIds;
// don't forget to forward factory function
EnhancedProto.newFromDom = function( subject, options, toolbar
) {
@@ -392,9 +404,9 @@
},
/**
- * Returns a list of sites already represented with a value.
+ * Returns a list of site IDs already used in a site link of this
SiteLinkEditTool.
*
- * @return {String[]} Array of site ids
+ * @return {String[]} Array of site IDs
*/
getRepresentedSiteIds: function() {
var sites = [];
@@ -410,6 +422,29 @@
},
/**
+ * Returns a list of site IDs which can be used but have not yet been
used in a site link of
+ * this SiteLinkEditTool.
+ *
+ * @since 0.4
+ *
+ * @return {String[]} Array of site IDs
+ */
+ getUnusedAllowedSiteIds: function() {
+ var allowedSites = this.getOption( 'allowedSites' ), // can be
object or array
+ representedSiteIds = this.getRepresentedSiteIds(),
+ unusedAllowedSiteIds = [];
+
+ for( var i in allowedSites ) {
+ var allowedSiteId = allowedSites[i].getId();
+
+ if( $.inArray( allowedSiteId, representedSiteIds ) ===
-1 ) {
+ unusedAllowedSiteIds.push( allowedSiteId );
+ }
+ }
+ return unusedAllowedSiteIds;
+ },
+
+ /**
* @see wb.ui.PropertyEditTool._getCounterNodes
*
* @return jQuery
diff --git a/lib/tests/qunit/wikibase.ui.SiteLinksEditTool.tests.js
b/lib/tests/qunit/wikibase.ui.SiteLinksEditTool.tests.js
index 153c924..be4f2a3 100644
--- a/lib/tests/qunit/wikibase.ui.SiteLinksEditTool.tests.js
+++ b/lib/tests/qunit/wikibase.ui.SiteLinksEditTool.tests.js
@@ -7,10 +7,10 @@
* @ingroup WikibaseLib
*
* @licence GNU GPL v2+
- * @author Daniel Werner
+ * @author Daniel Werner < [email protected] >
*/
-( function( mw, wb, $, QUnit, undefined ) {
+( function( mw, wb, $, QUnit ) {
'use strict';
var config = {
@@ -22,7 +22,7 @@
pageUrl: 'http://en.wikipedia.org/wiki/$1',
shortName: 'English',
languageCode: 'en',
- globalSiteIt: 'enwiki'
+ globalSiteId: 'enwiki'
},
de: {
apiUrl: 'http://de.wikipedia.org/w/api.php',
@@ -39,20 +39,144 @@
QUnit.module( 'wikibase.ui.SiteLinksEditTool', QUnit.newWbEnvironment( {
config: config,
setup: function() {
-
- this.apiResponse = {
- entity: { sitelinks: { dewiki: { title:
'ein_titel' } } }
- };
-
// get empty nodes we get when no links on the site yet:
- var dom = wb.ui.SiteLinksEditTool.getEmptyStructure();
+ var $dom = wb.ui.SiteLinksEditTool.getEmptyStructure();
// initialize:
- this.subject = new wb.ui.SiteLinksEditTool( dom );
+ this.subject = new wb.ui.SiteLinksEditTool( $dom, {
+ allowedSites: wb.getSites() // TODO: decouple
test from abusing global config here
+ } );
},
- teardown: function() {}
+ teardown: function() {
+ this.subject.destroy();
+ }
} ) );
+ /**
+ * Allows to enter a value into the SiteLinksEditTool which can then be
saved by calling
+ * stopEditing( true ) on the returned EditableValue. This is necessary
because of some ugly
+ * fake API requests have to be mocked to do this.
+ *
+ * @param editTool
+ * @param siteLink
+ * @param siteLinkGlobalId
+ * @returns {wb.ui.PropertyEditTool.EditableValue}
+ */
+ function hackyValueInsertion( editTool, siteLink, siteLinkGlobalId ) {
+ var initialValue = siteLink,
+ newValue = editTool.enterNewValue( initialValue );
+
+ // override AJAX API call
+ newValue.triggerApi = function( deferred, apiAction ) {
+ deferred.resolve( {} );
+ };
+
+ // set result set for validation
+ newValue.sitePageInterface.setResultSet( siteLink[1] );
+
+ // pretend API success
+ var fakeApiResponse = { entity: { sitelinks: {} } };
+ fakeApiResponse.entity.sitelinks[ siteLinkGlobalId ] = { title:
'whatever_title' };
+
+ newValue.triggerApi = function( deferred, apiAction ) {
+ deferred.resolve( fakeApiResponse );
+ };
+ return newValue;
+ }
+
+ /**
+ * Just like hackyValueInsertion but does the stopEditing as well.
+ *
+ * @see hackyValueInsertion
+ */
+ function hackyValueInsertionAndSave( editTool, siteLink,
siteLinkGlobalId ) {
+ var newValue = hackyValueInsertion.apply( null, arguments );
+
+ // TODO: stopEditing() should not fail for the 2nd value when
not doing this
+ var realTablesorter = $.fn.tablesorter;
+ $.fn.tablesorter = $.noop;
+
+ newValue.stopEditing( true );
+
+ $.fn.tablesorter = realTablesorter;
+
+ return newValue;
+ }
+
+ QUnit.test( 'getUnusedAllowedSiteIds()', function( assert ) {
+ var subject = this.subject;
+
+ assert.deepEqual(
+ subject.getUnusedAllowedSiteIds(),
+ [ 'en', 'de' ],
+ 'all allowed site links are returned as unused by newly
initialized edit tool'
+ );
+
+ hackyValueInsertionAndSave( subject, [ 'en', 'London' ],
'enwiki' );
+
+ assert.deepEqual(
+ subject.getUnusedAllowedSiteIds(),
+ [ 'de' ],
+ 'after site has been used, the site will not be
returned anymore'
+ );
+
+ hackyValueInsertionAndSave( subject, [ 'de', 'Berlin' ],
'dewiki' );
+
+ assert.deepEqual(
+ subject.getUnusedAllowedSiteIds(),
+ [],
+ 'after all sites are used, an empty array will be
returned'
+ );
+ } );
+
+ QUnit.test( 'getUnusedAllowedSiteIds()', function( assert ) {
+ var subject = this.subject;
+
+ assert.deepEqual(
+ subject.getRepresentedSiteIds(),
+ [],
+ 'initially, no site links, so no sites represented yet'
+ );
+
+ hackyValueInsertionAndSave( subject, [ 'en', 'London' ],
'enwiki' );
+
+ assert.deepEqual(
+ subject.getRepresentedSiteIds(),
+ [ 'en' ],
+ 'after site has been used, the site will be returned'
+ );
+
+ hackyValueInsertionAndSave( subject, [ 'de', 'Berlin' ],
'dewiki' );
+
+ assert.deepEqual(
+ subject.getRepresentedSiteIds(),
+ [ 'en', 'de' ],
+ 'after another site link has been entered, both sites
will be returned'
+ );
+ } );
+
+ QUnit.test( 'isFull()', function( assert ) {
+ var subject = this.subject;
+
+ assert.ok(
+ !subject.isFull(),
+ 'not full initially'
+ );
+
+ hackyValueInsertionAndSave( subject, [ 'de', 'Berlin' ],
'dewiki' );
+
+ assert.ok(
+ !subject.isFull(),
+ 'not full after entering one site link'
+ );
+
+ hackyValueInsertionAndSave( subject, [ 'en', 'London' ],
'enwiki' );
+
+ assert.ok(
+ subject.isFull(),
+ 'full after entering site links for all available sites'
+ );
+ } );
QUnit.test( 'adding a new editable site link', function( assert ) {
@@ -61,29 +185,16 @@
'editable values initiated correctly'
);
- var initialValue = [ 'Deutsch (de)', 'Berlin' ];
- var newValue = this.subject.enterNewValue( initialValue );
+ var rawNewValue = [ 'Deutsch (de)', 'Berlin' ];
+ var newValue = hackyValueInsertion( this.subject, rawNewValue,
'dewiki' );
- // override AJAX API call
- this.subject._editableValues[0].triggerApi = function(
deferred, apiAction ) {
- deferred.resolve( {} );
- };
-
- // set result set for validation
- this.subject._editableValues[0].sitePageInterface.setResultSet(
['Berlin'] );
-
- // pretend API success
- newValue.triggerApi = $.proxy( function( deferred, apiAction ) {
- deferred.resolve( this.apiResponse );
- }, this );
-
- assert.equal(
+ assert.strictEqual(
this.subject.getValues().length,
0,
'getValues() should return no elements since the new
one is still pending'
);
- assert.equal(
+ assert.strictEqual(
this.subject.getValues( true ).length,
1,
'getValues( true ) should return the pending element'
@@ -108,27 +219,27 @@
'new value has the value set in enterNewValue( value )'
);
- assert.equal(
+ assert.strictEqual(
newValue.startEditing(),
false,
'start editing already active, call function again'
);
- assert.equal(
+ assert.strictEqual(
newValue.stopEditing( true ).promisor.apiAction,
newValue.API_ACTION.SAVE,
'stopped editing (save), true returned because value
has changed (it was created)'
);
- assert.equal(
-
this.subject.enterNewValue().siteIdInterface.getResultSetMatch( initialValue[0]
),
+ assert.strictEqual(
+
this.subject.enterNewValue().siteIdInterface.getResultSetMatch( rawNewValue ),
null,
- 'The site id set already shouldn\'t be available in the
set of suggestions anymore'
+ 'The site id set already should not be available in the
set of suggestions anymore'
);
this.subject.destroy();
- assert.equal(
+ assert.strictEqual(
this.subject._editableValues,
null,
'destroyed editable values'
diff --git a/repo/resources/wikibase.ui.entityViewInit.js
b/repo/resources/wikibase.ui.entityViewInit.js
index 53be11a..83be1d8 100644
--- a/repo/resources/wikibase.ui.entityViewInit.js
+++ b/repo/resources/wikibase.ui.entityViewInit.js
@@ -141,7 +141,9 @@
)
);
// actual initialization
- new wb.ui.SiteLinksEditTool( $( this ) );
+ new wb.ui.SiteLinksEditTool( $( this ), {
+ allowedSites: wb.getSitesOfGroup( group
)
+ } );
} );
// BUILD TOOLBARS
--
To view, visit https://gerrit.wikimedia.org/r/67394
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I6524c4f1c97b072e08cec05c67e8b7e4b3b4b06d
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
