Jonas Kress (WMDE) has uploaded a new change for review. ( 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/SparqlQuery.test.js 4 files changed, 145 insertions(+), 37 deletions(-) git pull ssh://gerrit.wikimedia.org:29418/wikidata/query/gui refs/changes/55/364255/1 diff --git a/style.css b/style.css index 113da69..9dbcde1 100644 --- a/style.css +++ b/style.css @@ -419,7 +419,7 @@ } /* - Visual Editor + Query Helper */ .query-helper-trigger { display: none; @@ -455,6 +455,16 @@ padding: 10px; } +#query-box .query-helper .toolbar { + padding-left: 1.5em; + text-align: right; + opacity: 0.3; +} + +#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..3cbe2f0 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 @@ -422,7 +422,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 ); }; /** @@ -523,23 +551,6 @@ } }, { 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 }, @@ -596,4 +607,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..ea24b86 100644 --- a/wikibase/queryService/ui/queryHelper/SparqlQuery.js +++ b/wikibase/queryService/ui/queryHelper/SparqlQuery.js @@ -101,11 +101,13 @@ return false; } - if ( this._query.variables[0] === '*' && name.startsWith( '?' ) ) { - return true; - } - return this._query.variables.indexOf( name ) >= 0; + }; + + /** + * Check whether query uses wildcard SELECT * + */ + SELF.prototype.isWildcardQuery = function( name ) { }; /** @@ -115,15 +117,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._query.variables[0] === '*' ) { return this; } @@ -142,7 +144,7 @@ return; } - if ( this._query.variables.length === 1 && this._query.variables[0] === '*' ) { + if ( this._query.variables[0] === '*' ) { return; } @@ -150,6 +152,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 +307,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/SparqlQuery.test.js b/wikibase/tests/queryService/ui/queryHelper/SparqlQuery.test.js index df41033..23f7463 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,36 @@ 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, 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 cleand 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 cleand 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' ); + } ); + + }( jQuery, QUnit, sinon, wikibase ) ); -- To view, visit https://gerrit.wikimedia.org/r/364255 To unsubscribe, visit https://gerrit.wikimedia.org/r/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I27e19c68fffd0e2b98fd124cb1e3e7e45efc03b4 Gerrit-PatchSet: 1 Gerrit-Project: wikidata/query/gui Gerrit-Branch: master Gerrit-Owner: Jonas Kress (WMDE) <jonas.kr...@wikimedia.de> _______________________________________________ MediaWiki-commits mailing list MediaWiki-commits@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits