Do not expect the id field in DataSet to always be "id" (visjs#1293)

The DataSet allows a configurable "fieldId" argument which by default is the value "id". The checks for DataSet/DataView interfaces should use this id property name and not expect a fixed hardcoded value "id". Co-authored-by: Yotam Berkowitz <[email protected]>
bg · Nov 25, 2021 · 327fd3d · 327fd3d
1 parent 06f621b
commit 327fd3d
Show file tree

Hide file tree

Showing 6 changed files with 46 additions and 17 deletions.
diff --git a/lib/timeline/Graph2d.js b/lib/timeline/Graph2d.js
@@ -1,6 +1,6 @@
 import moment from '../module/moment';
-import util, { typeCoerceDataSet } from '../util';
-import { DataSet, DataView, isDataViewLike } from 'vis-data/esnext';
+import util, { typeCoerceDataSet, isDataViewLike } from '../util';
+import { DataSet, DataView } from 'vis-data/esnext';
 import Range from './Range';
 import Core from './Core';
 import TimeAxis from './component/TimeAxis';
@@ -26,7 +26,7 @@ import Configurator from '../shared/Configurator';
  */
 function Graph2d (container, items, groups, options) {
   // if the third element is options, the forth is groups (optionally);
-  if (!(Array.isArray(groups) || isDataViewLike("id", groups)) && groups instanceof Object) {
+  if (!(Array.isArray(groups) || isDataViewLike(groups)) && groups instanceof Object) {
     var forthArgument = options;
     options = groups;
     groups = forthArgument;
@@ -198,7 +198,7 @@ Graph2d.prototype.setItems = function(items) {
   if (!items) {
     newDataSet = null;
   }
-  else if (isDataViewLike("id", items)) {
+  else if (isDataViewLike(items)) {
     newDataSet = typeCoerceDataSet(items);
   }
   else {
@@ -236,7 +236,7 @@ Graph2d.prototype.setGroups = function(groups) {
   if (!groups) {
     newDataSet = null;
   }
-  else if (isDataViewLike("id", groups)) {
+  else if (isDataViewLike(groups)) {
     newDataSet = groups;
   }
   else {

diff --git a/lib/timeline/Timeline.js b/lib/timeline/Timeline.js
@@ -1,6 +1,6 @@
 import moment from '../module/moment';
-import util, { typeCoerceDataSet } from '../util';
-import { DataSet, DataView, isDataViewLike } from 'vis-data/esnext';
+import util, { typeCoerceDataSet, isDataViewLike } from '../util';
+import { DataSet, DataView } from 'vis-data/esnext';
 import Range from './Range';
 import Core from './Core';
 import TimeAxis from './component/TimeAxis';
@@ -37,7 +37,7 @@ export default class Timeline extends Core {
     }
 
     // if the third element is options, the forth is groups (optionally);
-    if (!(Array.isArray(groups) || isDataViewLike("id", groups)) && groups instanceof Object) {
+    if (!(Array.isArray(groups) || isDataViewLike(groups)) && groups instanceof Object) {
       const forthArgument = options;
       options = groups;
       groups = forthArgument;
@@ -317,7 +317,7 @@ export default class Timeline extends Core {
     if (!items) {
       newDataSet = null;
     }
-    else if (isDataViewLike("id", items)) {
+    else if (isDataViewLike(items)) {
       newDataSet = typeCoerceDataSet(items);
     }
     else {

diff --git a/lib/timeline/component/ItemSet.js b/lib/timeline/component/ItemSet.js
@@ -1,6 +1,6 @@
 import Hammer from '../../module/hammer';
-import util, { typeCoerceDataSet, randomUUID } from '../../util';
-import { DataSet, DataView, isDataViewLike } from 'vis-data/esnext';
+import util, { typeCoerceDataSet, randomUUID, isDataViewLike } from '../../util';
+import { DataSet, DataView } from 'vis-data/esnext';
 import TimeStep from '../TimeStep';
 import Component from './Component';
 import Group from './Group';
@@ -1000,7 +1000,7 @@ class ItemSet extends Component {
     if (!items) {
       this.itemsData = null;
     }
-    else if (isDataViewLike("id", items)) {
+    else if (isDataViewLike(items)) {
       this.itemsData = typeCoerceDataSet(items);
     }
     else {
@@ -1071,7 +1071,7 @@ class ItemSet extends Component {
     if (!groups) {
       this.groupsData = null;
     }
-    else if (isDataViewLike("id", groups)) {
+    else if (isDataViewLike(groups)) {
       this.groupsData = groups;
     }
     else {

diff --git a/lib/timeline/component/LineGraph.js b/lib/timeline/component/LineGraph.js
@@ -1,6 +1,6 @@
-import util, { typeCoerceDataSet, randomUUID } from '../../util';
+import util, { typeCoerceDataSet, randomUUID, isDataViewLike } from '../../util';
 import * as DOMutil from '../../DOMutil';
-import { DataSet, DataView, isDataViewLike } from 'vis-data/esnext';
+import { DataSet, DataView } from 'vis-data/esnext';
 import Component from './Component';
 import DataAxis from './DataAxis';
 import GraphGroup from './GraphGroup';
@@ -251,7 +251,7 @@ LineGraph.prototype.setItems = function (items) {
   if (!items) {
     this.itemsData = null;
   }
-  else if (isDataViewLike("id", items)) {
+  else if (isDataViewLike(items)) {
     this.itemsData = typeCoerceDataSet(items);
   }
   else {
@@ -312,7 +312,7 @@ LineGraph.prototype.setGroups = function (groups) {
   if (!groups) {
     this.groupsData = null;
   }
-  else if (isDataViewLike("id", groups)) {
+  else if (isDataViewLike(groups)) {
     this.groupsData = groups;
   }
   else {

diff --git a/lib/util.js b/lib/util.js
@@ -4,11 +4,26 @@ export * from "vis-util/esnext";
 import * as util from "vis-util/esnext";
 import { getType, isNumber, isString } from "vis-util/esnext";
 import { DataSet, createNewDataPipeFrom } from "vis-data/esnext";
+import {isDataViewLike as isDataViewLikeUpstream} from "vis-data/esnext";
 
 import moment from "moment";
 import xssFilter from 'xss';
 
 export { v4 as randomUUID } from "uuid";
+/**
+ * Test if an object implements the DataView interface from vis-data.
+ * Uses the idProp property instead of expecting a hardcoded id field "id".
+ */
+export function isDataViewLike(obj) {
+    if(!obj) {
+        return false;
+    }
+    let idProp = obj._idProp;
+    if(!idProp) {
+        return false;
+    }
+    return isDataViewLikeUpstream(idProp, obj);
+}
 
 // parse ASP.Net Date pattern,
 // for example '/Date(1198908717056)/' or '/Date(1198908717056-0700)/'

diff --git a/test/Timeline.test.js b/test/Timeline.test.js
@@ -1,3 +1,4 @@
+import assert from 'assert'
 import jsdom_global from 'jsdom-global'
 import { DataSet } from'vis-data/esnext'
 import Timeline from'../lib/timeline/Timeline'
@@ -50,4 +51,17 @@ describe('Timeline', () => {
 
 		timeline.setItems(null);
 	});
+	it("setItems(with custom ID property) should work", function() {
+		const timeline = new Timeline(document.createElement("div"), []);
+                const events = [
+			{start: new Date(), fooid: 1},
+			{start: new Date(), fooid: 2}
+		]
+		const dataSet = new DataSet(events,{fieldId:"fooid"});
+		timeline.setItems(dataSet);
+		timeline.setSelection([2], {animation: false});
+		const selectedIds = timeline.getSelection();
+		assert( selectedIds.length === 1 )
+		assert( dataSet.get(selectedIds[0]).fooid === 2 )
+	});
 });