Functional Issue also Best practices with deleting edges

For some reason, I’m seeing an error during deletion of an edge. Edges are not being deleted from the view when I override the the command handler, which caused me to add this code. The data appears to be in the model using the check, the error seems to be an RXJS type error. However, I’m not sure why I would have to do this, I would expect that the prototype call at the end would pass the delete command to the CommandHandler parent. Note that the selection of a node does work consistently, which doesn’t manually remove data from the model. The selection and deletion of an edge sometimes works, most of the time doesn’t.

This issue has prompted me to also ask what are the best practices for the situation below that show up in other usages of the diagram tool. Questions:

  1. Am I using containsLinkData correctly?
  2. I am determining if the selected object exists, and then qualifying it as type by looking for a field in the object’s data. Is there a better way to do this?
  3. Is there a macro that detects any type of key delete request as opposed to ORing all the keystroke options?

I’m using the two types differentiation to remove data from local storage and remote storage.

core.js:1440 ERROR TypeError: Cannot read property 'next' of undefined
at Q.removeLinkDataCollection (go.js:393)
at sg.DiagramEditorComponent.diagram.commandHandler.doKeyDown (diagram-editor.component.ts:106)

this.diagram.commandHandler.doKeyDown = () => {
const e = this.diagram.lastInput;
const cmd = this.diagram.commandHandler;

       if (e.key === 'Backspace' || e.key === 'Delete') {
          const object = e.diagram.selection.first();
          if (object && object.data['item_node']) {
              console.log('Delete request node');
        } else if (object) {
          console.log('Delete request - edge ' + JSON.stringify(object.data));
          if ((this.diagram.model as go.GraphLinksModel).containsLinkData(object.data)) {
                  (this.diagram.model as go.GraphLinksModel).removeLinkDataCollection(object.data);// LINE 106 WHERE ERROR IS
          } 
        }
    go.CommandHandler.prototype.doKeyDown.call(cmd);
}

CommandHandler.doKeyDown includes this code:

  . . .
  } else if (key === 'Del' || key === 'Backspace') {
    if (this.canDeleteSelection()) this.deleteSelection();
  } . . .

So it would make more sense for you to override the CommandHandler.deleteSelection method in order to customize the behavior of when the user deletes some Parts.

In any case your override of CommandHandler.doKeyDown should not be trying to do any modification of the model or the diagram if it is going to call the super method anyway, because it calls CommandHandler.deleteSelection.

On a different issue, GraphLinksModel.removeLinkDataCollection needs to take a collection (an Iterable or an Array), not an individual link data object. Hence no next method of an undefined iterator.

Walter, the change in override was effective, and I removed the if statement. I’m still seeing the same issue with the edge not being removed from the view though. I tried both calling removeLinkDataCollection and calling the super method. There is no error reported.

Why are you trying to do anything, when CommandHandler.deleteSelection makes all of the model changes that you need?

There are two reasons for your astute ‘do anything’ question - one is the original problem, which I interpreted incorrectly as the edges weren’t being deleted, there were actually multiple edges. The second is that I have to eventually qualify the deletion and take other actions.

What I discovered is that there is the library we are using is emitting data different in different contexts, which resulted in multiple add edge requests. I used findLinksBetween as an existence check to qualify drawing an edge until that error is resolved. The reason nodes weren’t affected was that the diagram is cleared before drawing nodes, and then the edges are rendered.

There are a few other actions I’m doing when modifying the diagram. I’m going to need to determine when a user has permission to delete, which can be time dependent as well as role type permissions, so it’s on the fly. I currently modify an internal repository for the objects which caches all the data in the application. I send a request to the database if needed. Maybe there’s a better way to do that.

The gojs documentation is really phenomenal. It’s possible I could be coming at this from an incorrect entry point that just works, which is why I question if I’m using best practices. I knew I had an error, but at the same time in understanding the elegance of my gojs implementation in other ways, this felt clumsy and un-necessary.

Well, you know about the Part.deletable property, which you could set or bind if the times when the value might change are limited to within a transaction.

The deleteSelection command calls Part.canDelete to decide if a Part may be deleted by the user. I suppose you could override that method if you really need to decide “at-the-moment”. But if you do, I’d be worried about the value of the predicate changing unexpectedly, if the predicate is called more than once for various reasons.

Yes, I agree that’s a better decision. I probably spoke too soon before, I’m seeing issues with the link not being removed from the model, although it is removed from the view. In attempting to debug with Angular, I see some unexpected behavior. Why is links count not zero?

That being said, I thought ‘go.CommandHandler.prototype.deleteSelection.call(cmd);’ would handle the visual removal as well as the model removal. While I now see the view being modified - the model remains unchanged.

        const object = e.diagram.selection.first();
          if (object && object.data['edge']) { //modified for brevity
            const link = object.data['edge'];
            const sourceNode = e.diagram.findNodeForKey(link['source_id']); 
            const sinkNode = e.diagram.findNodeForKey(link['sink_id']);
            let links = sourceNode.findLinksBetween(sinkNode); 
            console.log('NUM Links before: ' + links.count); //expect 1, got 1
            (e.diagram.model as go.GraphLinksModel).removeLinkData(object);
            links = sourceNode.findLinksBetween(sinkNode); //expect 0, got 1
            console.log('NUM Links after: ' + links.count);

NUM Links before: 1
NUM Links after: 1

In this scenario, has the user selected a Link?

e.diagram.selection is a collection of Parts, so e.diagram.selection.first() will return either null or a Part.

So your call to Model.removeLinkData is passing a Part (can’t be null there), rather than the model data object. Hence you should be passing object.data to that method, instead of just object.

But I’ll ask the obvious question: why bother to find the Nodes and Links in the Diagram? if you have a link data object, you can just call GraphLinkModel.removeLinkData on it.

I’ll speculate that if there might be multiple links between the same pair of nodes, you might want to delete a particular link. You could do that by directly looking at the links connecting the nodes connected by the selected link:

const link = e.diagram.selection.first();
if (link !== null) {
    const links = link.fromNode.findLinksBetween(link.toNode);
    console.log(links.count);
    . . .
    // make a change in the diagram:
    e.diagram.commit(d => d.remove(link), "removed selected link");
}

For that last statement you could do an equivalent at the model level:

    // make a change in the model:
    e.diagram.model.commit(m => m.removeLinkData(link.data), "removed selected link");

Correct, I misread that. The model removal functions correctly now.

When a delete key event fires my overloaded deleteSelection, I have to determine which object has been deleted so I can modify the database. Is there something I’m not aware of?

Thanks for that code. Those types of insights elude me when reading the docs.

The most general thing to do is implement a Model Changed listener and look for ChangedEvent.isTransactionFinished. When it’s true, the object is a Transaction (or null), and then you can scan the Transaction.changes list for ChangedEvents that have ChangedEvent.model equal to your model and are of interest to you for saving to the database.

The UpdateDemo does that (and more). Also read: GoJS Changed Events -- Northwoods Software