jenkins-bot has submitted this change and it was merged. (
https://gerrit.wikimedia.org/r/364255 )
Change subject: Refactor Query Helper delete
......................................................................
Refactor Query Helper delete
Move delete button from popover to table column.
Also modifies variable handling to better cope with 'SELECT *'.
Change-Id: I27e19c68fffd0e2b98fd124cb1e3e7e45efc03b4
---
M style.css
M wikibase/queryService/ui/queryHelper/QueryHelper.js
M wikibase/queryService/ui/queryHelper/SparqlQuery.js
M wikibase/tests/queryService/ui/queryHelper/QueryHelper.test.js
M wikibase/tests/queryService/ui/queryHelper/SparqlQuery.test.js
5 files changed, 172 insertions(+), 41 deletions(-)
Approvals:
Jonas Kress (WMDE): Looks good to me, but someone else must approve
Lucas Werkmeister (WMDE): Looks good to me, approved
jenkins-bot: Verified
diff --git a/style.css b/style.css
index 113da69..084c8b7 100644
--- a/style.css
+++ b/style.css
@@ -419,7 +419,7 @@
}
/*
- Visual Editor
+ Query Helper
*/
.query-helper-trigger {
display: none;
@@ -455,6 +455,20 @@
padding: 10px;
}
+#query-box .query-helper .toolbar {
+ padding-left: 1.5em;
+ text-align: right;
+ opacity: 0.3;
+}
+
+#query-box .query-helper .toolbar .fa {
+ font-size: 1.2em;
+}
+
+#query-box .query-helper tr:hover > .toolbar {
+ opacity: 1;
+}
+
.select2-container {
z-index: 2000;
min-width: 200px;
diff --git a/wikibase/queryService/ui/queryHelper/QueryHelper.js
b/wikibase/queryService/ui/queryHelper/QueryHelper.js
index 474ed2c..d2fbb0a 100644
--- a/wikibase/queryService/ui/queryHelper/QueryHelper.js
+++ b/wikibase/queryService/ui/queryHelper/QueryHelper.js
@@ -3,7 +3,7 @@
wikibase.queryService.ui = wikibase.queryService.ui || {};
wikibase.queryService.ui.queryHelper = wikibase.queryService.ui.queryHelper ||
{};
-wikibase.queryService.ui.queryHelper.QueryHelper = ( function( $, wikibase ) {
+wikibase.queryService.ui.queryHelper.QueryHelper = ( function( $, wikibase, _
) {
'use strict';
var FILTER_PREDICATES = {
@@ -306,7 +306,7 @@
var variable = self._query.getBoundVariables().shift();
if ( !variable ) {
- variable = '?' + '_' + name.replace( /(
|[^a-z0-9])/gi, '_' );
+ variable = '?' + name.replace( /(
|[^a-z0-9])/gi, '_' );
}
var prop = 'http://www.wikidata.org/prop/direct/' + (
data && data[id] && data[id].propertyId || 'P31' );// FIXME technical debt
@@ -372,7 +372,8 @@
if ( this._isSimpleMode && this._isInShowSection( triple ) &&
( this._query.hasVariable( triple.object ) ===
false &&
- this._query.hasVariable(
triple.object + 'Label' ) === false ) ) {
+ this._query.hasVariable(
triple.object + 'Label' ) === false &&
+ this._query.isWildcardQuery()
=== false ) ) {
return true;
}
@@ -422,7 +423,35 @@
}
} );
- return $triple;
+ return $triple.append( this._getTripleHtmlToolbar( $triple,
triple ) );
+ };
+
+ /**
+ * @private
+ */
+ SELF.prototype._getTripleHtmlToolbar = function( $triple, triple ) {
+ var self = this;
+
+ var $delete = $( '<a href="#">' ).addClass( 'fa fa-trash-o'
).click( function () {
+ var variables = _.compact( triple.triple ).filter(
function ( v ) {
+ return typeof v === 'string' &&
v.startsWith( '?' );
+ } ),
+ variablesLabels = variables.map( function ( v )
{
+ return v + 'Label';
+ } );
+
+ triple.remove();
+ $triple.remove();
+ self._query.cleanupVariables( variables.concat(
variablesLabels ) );
+
+ if ( self._changeListener ) {
+ self._changeListener( self );
+ }
+
+ return false;
+ } );
+
+ return $( '<td class="toolbar">' ).append( $delete );
};
/**
@@ -522,27 +551,6 @@
self._changeListener( self );
}
}, {
- trash: function() {
- triple.remove();
-
- var variable = triple.triple.object;
- if ( triple.triple.object === entity ||
- (
triple.triple.object.startsWith( '?' ) === false && triple.triple.predicate ===
entity ) ) {
- variable =
triple.triple.subject;
- }
- $label.closest( 'tr' ).remove();
-
- if (
self._query.getBoundVariables().indexOf( variable ) === -1 ) {
- self._query.removeVariable(
variable );
- self._query.removeVariable(
variable + 'Label' );
- }
-
- if ( self._changeListener ) {
- self._changeListener( self );
- }
- $( '.tooltip' ).hide();
- return true;//close popover
- },
tag: function() {
if ( triple.triple.object.startsWith(
'?' ) ) {
self._query
@@ -596,4 +604,4 @@
};
return SELF;
-}( jQuery, wikibase ) );
+}( jQuery, wikibase, _ ) );
diff --git a/wikibase/queryService/ui/queryHelper/SparqlQuery.js
b/wikibase/queryService/ui/queryHelper/SparqlQuery.js
index 8276100..99f3e2d 100644
--- a/wikibase/queryService/ui/queryHelper/SparqlQuery.js
+++ b/wikibase/queryService/ui/queryHelper/SparqlQuery.js
@@ -101,11 +101,18 @@
return false;
}
- if ( this._query.variables[0] === '*' && name.startsWith( '?' )
) {
+ return this._query.variables.indexOf( name ) >= 0;
+ };
+
+ /**
+ * Check whether query uses wildcard SELECT *
+ */
+ SELF.prototype.isWildcardQuery = function() {
+ if ( this._query.variables && this._query.variables[0] === '*'
) {
return true;
}
- return this._query.variables.indexOf( name ) >= 0;
+ return false;
};
/**
@@ -115,15 +122,15 @@
* @return {wikibase.queryService.ui.queryHelper.SparqlQuery}
*/
SELF.prototype.addVariable = function( name ) {
- if ( !name.startsWith( '?' ) ) {
- return this;
- }
-
- if ( this._query.variables.length === 1 &&
this._query.variables[0] === '*' ) {
+ if ( !name || !name.startsWith( '?' ) ) {
return this;
}
if ( this._query.variables.indexOf( name ) >= 0 ) {
+ return this;
+ }
+
+ if ( this.isWildcardQuery() ) {
return this;
}
@@ -142,7 +149,7 @@
return;
}
- if ( this._query.variables.length === 1 &&
this._query.variables[0] === '*' ) {
+ if ( this.isWildcardQuery() ) {
return;
}
@@ -150,6 +157,38 @@
if ( index >= 0 ) {
this._query.variables.splice( index, 1 );
}
+
+ if ( this._query.variables.length === 0 ) {
+ this._query.variables.push( '*' );
+ }
+ };
+
+ /**
+ * Removes unused variables from the query SELECT
+ * Disclaimer: may remove too much
+ *
+ * @param {string[]} [variables] cleanup only variables in this list
+ */
+ SELF.prototype.cleanupVariables = function( variables ) {
+ var self = this,
+ usedVariables = this.getTripleVariables();
+
+ // TODO check sub queries
+ var toRemove = this._query.variables.filter( function (
variable ) {
+ if ( variables && variables.indexOf( variable ) === -1
) {
+ return false;
+ }
+
+ if ( usedVariables.indexOf( variable ) === -1 ) {
+ return true;
+ }
+
+ return false;
+ } );
+
+ toRemove.map( function ( v ) {
+ self.removeVariable( v );
+ } );
};
/**
@@ -273,6 +312,30 @@
};
/**
+ * Get all variables used in triples
+ *
+ * @return {string[]}
+ */
+ SELF.prototype.getTripleVariables = function() {
+ var variables = {};
+
+ $.each( this.getTriples(), function( i, t ) {
+ if ( typeof t.triple.subject === 'string' &&
t.triple.subject.startsWith( '?' ) ) {
+ variables[t.triple.subject] = true;
+ }
+ if ( typeof t.triple.predicate === 'string' &&
t.triple.predicate.startsWith( '?' ) ) {
+ variables[t.triple.predicate] = true;
+ }
+ if ( typeof t.triple.object === 'string' &&
t.triple.object.startsWith( '?' ) ) {
+ variables[t.triple.object] = true;
+ }
+
+ } );
+
+ return Object.keys( variables );
+ };
+
+ /**
* Get variables that are bound to a certain value
*
* @return {string[]}
diff --git a/wikibase/tests/queryService/ui/queryHelper/QueryHelper.test.js
b/wikibase/tests/queryService/ui/queryHelper/QueryHelper.test.js
index c311bfe..4dd944c 100644
--- a/wikibase/tests/queryService/ui/queryHelper/QueryHelper.test.js
+++ b/wikibase/tests/queryService/ui/queryHelper/QueryHelper.test.js
@@ -39,8 +39,19 @@
sparqlIn: 'SELECT ?pid WHERE { BIND(wd:Q5 AS
?thing) ?pid wdt:P31 ?thing.}',
sparqlOut: 'SELECT ?pid WHERE {\n BIND(wd:Q5
AS ?thing)\n ?pid wdt:P31 ?thing.\n}',
text: 'instance of human Limit'
+ },
+ {
+ name: 'Wildcard query with unbound variables',
+ sparqlIn: 'SELECT * WHERE { ?item wdt:P31
?instance. }',
+ sparqlOut: 'SELECT * WHERE { ?item wdt:P31
?instance. }',
+ text: 'instance of Limit'
+ },
+ {
+ name: 'Wildcard query with show section and
unbound query',
+ sparqlIn: 'SELECT * WHERE { ?item wdt:P31
wd:Q146. OPTIONAL { ?item wdt:P39 ?position. } }',
+ sparqlOut: 'SELECT * WHERE {\n ?item wdt:P31
wd:Q146.\n OPTIONAL { ?item wdt:P39 ?position. }\n}',
+ text: 'instance of cat position held Limit'
}
-
];
var LABELS = {
diff --git a/wikibase/tests/queryService/ui/queryHelper/SparqlQuery.test.js
b/wikibase/tests/queryService/ui/queryHelper/SparqlQuery.test.js
index df41033..2c9eb10 100644
--- a/wikibase/tests/queryService/ui/queryHelper/SparqlQuery.test.js
+++ b/wikibase/tests/queryService/ui/queryHelper/SparqlQuery.test.js
@@ -8,6 +8,7 @@
SIMPLE: 'SELECT * WHERE {}',
LIMIT: 'SELECT * WHERE {} LIMIT 10',
VARIABLES: 'SELECT ?x1 ?x2 ?x3 WHERE {} LIMIT 10',
+ TRIPLE_VARIABLES: 'SELECT ?y1 ?y2 ?y3 WHERE { ?x1 ?x2 ?x3.
}\nLIMIT 10',
TRIPLES_UNION: 'SELECT ?x1 ?x2 ?x3 WHERE { <S> <P> <O>.
OPTIONAL{ <S1> <P1> <O1> } <S2> <P2> <O2>. { <SU1> <PU1> <OU1> } UNION { <SU2>
<PU2> <OU2> } }',
TRIPLES: 'SELECT ?x1 ?x2 ?x3 WHERE { <S> <P> <O>. OPTIONAL{
<S1> <P1> <O1> } <S2> <P2> <O2>.}',
SUBQUERIES: 'SELECT * WHERE { {SELECT * WHERE { {SELECT *
WHERE {}} }} }',
@@ -99,14 +100,10 @@
} );
QUnit.test( 'When query is \'' + QUERY.SIMPLE + '\' THEN', function(
assert ) {
- assert.expect( 6 );
-
var q = new PACKAGE.SparqlQuery();
q.parse( QUERY.SIMPLE );
- assert.ok( q.hasVariable( '?XX' ), '?XX must be a variable' );
- assert.ok( q.hasVariable( '?YYY' ), '?YYY must be a variable' );
- assert.ok( q.hasVariable( '?ZZLABEL' ), '?ZZLABEL must be a
variable' );
+ assert.notOk( q.hasVariable( '?XX' ), '?XX must not be a
variable' );
assert.notOk( q.hasVariable( 'XX' ), 'XX must not be a
variable' );
assert.notOk( q.hasVariable( 'YY' ), 'XX must not be a
variable' );
assert.notOk( q.hasVariable( 'ZZ' ), 'XX must not be a
variable' );
@@ -285,4 +282,42 @@
assert.equal( s[0].name, 'http://wikiba.se/ontology#label',
'Wikibase label service should be in services' );
} );
+ QUnit.test( 'When query is \'' + QUERY.TRIPLE_VARIABLES + '\'',
function( assert ) {
+ var q = new PACKAGE.SparqlQuery();
+ q.parse( QUERY.TRIPLE_VARIABLES );
+
+ assert.deepEqual( q.getTripleVariables(), [ '?x1', '?x2', '?x3'
], 'all variables should be returned' );
+ } );
+
+ QUnit.test( 'When query is \'' + QUERY.TRIPLE_VARIABLES + '\' and
variables are cleaned up ', function( assert ) {
+ var q = new PACKAGE.SparqlQuery();
+ q.parse( QUERY.TRIPLE_VARIABLES );
+ q.cleanupVariables();
+
+ assert.deepEqual( q.getQueryString(),
QUERY.TRIPLE_VARIABLES.replace( /SELECT(.*)WHERE/, 'SELECT * WHERE' ),
+ 'Unused variables should be removed' );
+ } );
+
+ QUnit.test( 'When query is \'' + QUERY.TRIPLE_VARIABLES + '\' and
variables are cleaned up with filter ', function( assert ) {
+ var q = new PACKAGE.SparqlQuery();
+ q.parse( QUERY.TRIPLE_VARIABLES );
+ q.cleanupVariables( [ '?someUnrelatedVariable' ] );
+
+ assert.deepEqual( q.getQueryString(), QUERY.TRIPLE_VARIABLES ,
'Unused variables in filter list should not be removed' );
+ } );
+
+ QUnit.test( 'When query is \'' + QUERY.SIMPLE + '\' ', function( assert
) {
+ var q = new PACKAGE.SparqlQuery();
+ q.parse( QUERY.SIMPLE );
+
+ assert.ok( q.isWildcardQuery(), 'isWildcardQuery returns true'
);
+ } );
+
+ QUnit.test( 'When query is \'' + QUERY.VARIABLES + '\' ', function(
assert ) {
+ var q = new PACKAGE.SparqlQuery();
+ q.parse( QUERY.VARIABLES );
+
+ assert.notOk( q.isWildcardQuery(), 'isWildcardQuery returns
false' );
+ } );
+
}( jQuery, QUnit, sinon, wikibase ) );
--
To view, visit https://gerrit.wikimedia.org/r/364255
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I27e19c68fffd0e2b98fd124cb1e3e7e45efc03b4
Gerrit-PatchSet: 5
Gerrit-Project: wikidata/query/gui
Gerrit-Branch: master
Gerrit-Owner: Jonas Kress (WMDE) <[email protected]>
Gerrit-Reviewer: Jonas Kress (WMDE) <[email protected]>
Gerrit-Reviewer: Lucas Werkmeister (WMDE) <[email protected]>
Gerrit-Reviewer: Smalyshev <[email protected]>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
