Number of columns for X and Y values for a graph keep increasing every time I open the dialog for empty <graph> node
Open, LowPublic1 Estimated Story Points
Actions

Assigned To

None

Authored By

	Ryasmeen
	Nov 21 2016, 10:40 PM

Description

Steps to reproduce:

Open VE
Go to Insert>Graph
Go to Data section
Delete all the values for x,y
Click on "Insert"
When an empty graph appears click on Edit again to open the dialog

Now observe that in the Data section there are now two extra columns for x and y. If you close and reopen again, there will be two more....

Screen Shot 2016-11-21 at 2.33.41 PM.png (735×1 px, 194 KB)

Details

Subject	Repo	Branch	Lines +/-
VE table widget is still a work in progress	mediawiki/extensions/Graph	REL1_31	+5 -1
VE table widget is still a work in progress	mediawiki/extensions/Graph	master	+5 -1
[Graph] Add non voting QUnit job	integration/config	master	+10 -1

Customize query in gerrit

Related Objects

Mentioned In: T198229: [Graph] extension fails QUnit tests due to timeout
Mentioned Here: T198229: [Graph] extension fails QUnit tests due to timeout

Event Timeline

Ryasmeen created this task.Nov 21 2016, 10:40 PM

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptNov 21 2016, 10:40 PM

Ryasmeen renamed this task from Number of columns for X and Y values for a graph keeps increasing every time I open the dialog for empty <graph> node to Number of columns for X and Y values for a graph keep increasing every time I open the dialog for empty <graph> node.Nov 21 2016, 10:40 PM

Jdforrester-WMF triaged this task as High priority.Nov 22 2016, 8:10 PM

Jdforrester-WMF moved this task from Backlog to VisualEditor on the MediaWiki-extensions-Graph board.

Jdforrester-WMF set the point value for this task to 1.

Jdforrester-WMF moved this task from To Triage to TR0: Interrupt on the VisualEditor board.

Jdforrester-WMF added a subscriber: • ferdbold.

Nice catch, I'll look into this ASaP

Is this still broken?

yes it is,

• Deskana lowered the priority of this task from High to Low.Aug 24 2017, 10:37 AM

• Deskana moved this task from TR0: Interrupt to Freezer on the VisualEditor board.

Change 437233 had a related patch set uploaded (by Hashar; owner: Hashar):
[integration/config@master] [Graph] Add non voting QUnit job

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

gerritbot added a project: Patch-For-Review.Jun 4 2018, 12:57 PM

Change 437233 merged by jenkins-bot:
[integration/config@master] [Graph] Add non voting QUnit job

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

I tried to add the QUnit tests in CI, the ve.ui.TableWidget tests fail when inserting a column. The first is one can not add an empty column, the test suite has:

modules/ve-graph/tests/ext.graph.visualEditor.test.js

592   widgetA.insertColumn();

Which tries to normalize the insertion of data to an empty array but the code fails later because data is not initialized:

modules/ve-graph/widgets/ve.dm.TableWidgetModel.js

ve.dm.TableWidgetModel.prototype.insertColumn = function ( data, index, key, label ) {
    ...
    // normalized to an empty array
    insertData = ( Array.isArray( data ) ) ? data : [];  
    ...
    // original 'data' is passed which is 'undefined'
    this.emit( 'insertColumn', data, index, key, label );

I changed the test to use:

widgetA.insertColumn( [ 'foo', 'bar' ] );

The assertion Row and column are added successfully then fails and show the column has been inserted twice.

From the git log, @ferdbold implemented the TableWidget but it is not entirely functional yet.

CI now runs the QUnit tests, though the job is non voting. An example run https://integration.wikimedia.org/ci/job/mwext-qunit-jessie-non-voting/1/console

  ext.graph.visualEditor
    ✔ ve.dm.MWGraphNode
    ✔ ve.ce.MWGraphNode
    ✖ ve.ce.MWGraphNode.static
    ✔ ve.dm.MWGraphModel
    ✔ ve.dm.MWGraphModel.static
    ✖ ve.ui.TableWidget


SUMMARY:
✔ 732 tests completed
✖ 2 tests failed

FAILED TESTS:
  ext.graph.visualEditor
    ✖ ve.ce.MWGraphNode.static
      Chrome 57.0.2987 (Linux 0.0.0)
    Test took longer than 60000ms; test timed out.
        at node_modules/qunit/qunit/qunit.js:2053:7
    

    ✖ ve.ui.TableWidget
      Chrome 57.0.2987 (Linux 0.0.0)
    TypeError: Cannot read property '0' of undefined
        at VeUiTableWidget.ve.ui.TableWidget.onInsertColumn 
        at VeDmTableWidgetModel.oo.EventEmitter.emit 
        at VeDmTableWidgetModel.ve.dm.TableWidgetModel.insertColumn
        at VeUiTableWidget.ve.ui.TableWidget.insertColumn

hashar mentioned this in T198229: [Graph] extension fails QUnit tests due to timeout.Jun 26 2018, 4:44 PM

Change 442145 had a related patch set uploaded (by Hashar; owner: Hashar):
[mediawiki/extensions/Graph@master] VE table widget is still a work in progress

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

Change 442145 merged by jenkins-bot:
[mediawiki/extensions/Graph@master] VE table widget is still a work in progress

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

The QUnit test ve.ui.TableWidget is now skipped until the feature is completed.

The ve.ce.MWGraphNode.static test is unrelated and is being fixed via T198229

Change 653994 had a related patch set uploaded (by Umherirrender; owner: Hashar):
[mediawiki/extensions/Graph@REL1_31] VE table widget is still a work in progress

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

gerritbot added a project: Patch-For-Review.Jan 4 2021, 6:08 PM

Change 653994 merged by Umherirrender:
[mediawiki/extensions/Graph@REL1_31] VE table widget is still a work in progress