Fix empty cycle point bug

[MetRonnie]

Closes #1922

Guard against node.ancestors being undefined rather than an empty array.

Check List

• I have read CONTRIBUTING.md and added my name as a Code Contributor.
• Contains logically grouped changes (else tidy your branch by rebase).
• Does not contain off-topic changes (use other PRs for other changes).
• Tests not needed?
• Changelog entry
• Docs not needed

[oliver-sanders]

Hmmm, this suggests that we are receiving an added delta for the family, in which ancestors is undefined? If so, we have a data store bug :(

Note, this is also relevant to the graph view which needs to request the family hierarchy.

By this logic:

• If ancestors is undefined on the first delta we receive, then we won't create the families under the cycle point.
• If ancestors is defined on a subsequent delta, that won't trigger this to subsequently run (because the node already exists in the store).

This will certainly give us a partial tree at first, but I'm not sure if something else in the store will subsequently force these nodes to be created on future deltas, so the issue might get fixed in time.

[MetRonnie]

I don't think there are any ancestors that are actually missing in the data. I've captured this in the wild running with the site default stock UI server. The delta in question seems to be an updated delta:

{
  "id": "3",
  "type": "data",
  "payload": {
    "data": {
      "deltas": {
        "added": {
          "familyProxies": [],
          "taskProxies": [],
          "jobs": []
        },
        "updated": {
          "workflow": { "id": "~user/wflow" },
          "familyProxies": [
            {
              "id": "~user/wflow//1/root",
              "state": "running",
              "childTasks": [
                { "id": "~user/wflow//1/one" },
                { "id": "~user/wflow//1/foo" },
                ...
              ]
            }
          ],
          "taskProxies": [...],
          "jobs": [...]
        },
        "pruned": {
          "familyProxies": [],
          "taskProxies": [],
          "jobs": []
        },
        "id": "~user/wflow"
      }
    }
  }
}

[MetRonnie]

What is missing is an added delta containing the familyProxies before the the updated delta

[oliver-sanders]

Right, so this fix would disable the logic which populates these entries then?

(the logic only kicks in when a node is created, not when it is updated?)

[MetRonnie]

Looks like the logic kicks in if an update is received but the family proxy is not yet in the index (as is the case with the delta above)

cylc-ui/src/store/workflows.module.js

Lines 348 to 359 in cb0d052

	let treeItem = getIndex(state, id)
	if (treeItem) {
	// node already exists -> update it
	collateState(state, treeItem, updatedData, false)
	Object.assign(treeItem.node, updatedData)
	applyInheritance(state, treeItem)
	return
	}
	// create a new tree item
	let treeParent
	[treeParent, treeItem] = createTreeNode(state, id, tokens, updatedData)

Sounds like this situation of missing an added delta is a cylc-flow/cylc-uiserver bug. In the meantime, is this UI fix good to go for 2.12.0?

[oliver-sanders]

(corrected understanding of the problem)

• There was no added delta for the FamilyProxy.
• Only an updated delta which did not have ancestors defined (which is reasonable for an updated delta).

Does this mean:

• The added delta just never got sent, something was lost in translation (family should be added to the store).
• The family was created and pruned, but an updated delta slipped through the net (family should not be added to the store).

?

[MetRonnie]

(Discussed in person after some digging, summarising here for the record)

Does this mean: The added delta just never got sent, something was lost in translation (family should be added to the store).

Yes, that. Possibly fixed by cylc/cylc-uiserver#761

N.B. updated deltas for root families will never have ancestors as we have updated(stripNull = true) in the tree view subscription.

Btw, this PR will also defend against the bug if stripNull is ever turned on for added deltas

[oliver-sanders]

Yes, that. Possibly fixed by cylc/cylc-uiserver#761

Hard to confirm this intermittent bug I appreciate, it sounds like this issue may have been fixed, do we think we still need this?

Btw, this PR will also defend against the bug if stripNull is ever turned on for added deltas

The stripNull option only makes sense in the context of updated deltas (shouldn't be an option for added) so we don't need to be robust against this (note the stripNull = false default is enforced by subscription merging so can't be done by accident).

[MetRonnie]

Don't know if it has been fixed by cylc/cylc-uiserver#761, and I don't think this PR will do any harm even if it has already been fixed. I've asked David Sutherland if he has any insight - cylc/cylc-uiserver#767

[oliver-sanders]

This PR is skipping some logic in an event which (by contract) should not be possible.

So I would be keen to avoid this workaround if possible (not 100% convinced on the won't do harm bit).

[MetRonnie]

Just experienced this in the wild again unfortunately

cylc-flow 8.6.3
cylc-uiserver 1.8.4
cylc-ui 2.13.0