add many search feature #653

MiccWan · 2022-01-19T10:08:03Z

fix GlobalErrorNode.getSelectedChildren
fix errors not recorded correctly in TraceCollection
fix showError should only traverse leaves
add SearchController in dbux-code
fix many search bugs
show search result in GlobalAnalysisView
focus SearchResultNode on search

- fix `SearchBar` focus race condition - fix wont search when press enter after mode switch - fix undisposed handler errors - merge search `SelectorType`s in `ContextNodeManager`

- add `DataNodeNode` - use `DataNodeNode` on `DataFlowView` - add `DataNode.applicationId` - `traceSelection.isSelected` supports nodeId

Domiii · 2022-01-23T08:32:54Z

dbux-code/src/codeUtil/treeView/ContextNode.js

+  }
+
+  handleClick = () => {
+    const { applicationId, contextId } = this.context;


@MiccWan add dp getter to ContextNode instead

Domiii · 2022-01-23T08:35:34Z

dbux-code/src/codeUtil/treeView/TraceNode.js

 import allApplications from '@dbux/data/src/applications/allApplications';
 import BaseTreeViewNode from './BaseTreeViewNode';

 export default class TraceNode extends BaseTreeViewNode {
-  get clickUserActionType() {


@MiccWan since this is part of Pathways: did you make sure whether pathways works as expected when making this change?
We generally want to avoid touching features that we are not currently working on them. If absolutely necessary, make the change, but then also make sure to test the corresponding changes.

Added getter of clickUserActionType which returns false to BaseTreeViewNode, so by default no UserEvent will be send on click.

Domiii · 2022-01-23T08:40:01Z

dbux-code/src/globalAnalysisView/GlobalAnalysisViewController.js

@@ -33,18 +36,34 @@ export default class GlobalAnalysisViewController {
    this.refresh();
  }

+  handleSearch = async () => {
+    await this.refresh();
+    this.focusOnSearchResult();


@MiccWan no await for this?
Since race conditions in the focus controller are one of our biggest recurring problems, can you briefly document the expectations of the potential main culprits, such as this one? Or maybe, we can have some SOP or recommended usage in a more centralized location that explains what will and what will not trigger race conditions?

My bad. There should be an await definitely.
I add this line before actually implement the function and forget to add the missing await after making it an async function.

Domiii · 2022-01-23T08:52:52Z

dbux-code/src/globalAnalysisView/nodes/GlobalErrorsChildNode.js

@@ -8,4 +8,11 @@ export default class GlobalErrorsChildNode extends TraceNode {
  buildChildren() {
    return this.childTraces.map(t => this.treeNodeProvider.buildNode(TraceNode, t, this));
  }
+
+  getSelectedChildren() {


Should this function not be in TraceNode?

TraceNodes have their own isSelected method. getSelectedChildren is for their parent but not TraceNode themself.

Domiii · 2022-01-23T08:54:09Z

dbux-code/src/globalAnalysisView/nodes/GlobalErrorsNode.js

@@ -33,6 +36,12 @@ export default class GlobalErrorsNode extends BaseTreeViewNode {
        return child;
      }
    }
+    for (const child of this.children) {


can use this.children.find instead

Cannot simply use find since it is a flatFind(or findMap) operation.

Domiii · 2022-01-23T08:58:29Z

dbux-code/src/search/searchController.js

+
+class SearchController {
+  constructor() {
+    this._emitter = new NanoEvents();


Since this is a singleton, have you made sure that it gets reset when it is invalidated?
Specifically: this stores matches, contexts etc. When their application is deselected/removed, are you making sure that those invalid data points are not lingering?
Can you add at least part of the reset mechanic as a method to this class, so we can easily find where it happens, when necessary?

Solution: re-search every time ApplicationSelectionChanged.

Domiii · 2022-01-23T09:06:00Z

dbux-data/src/dataProviderUtil.js

  },

-  findContextsByTraceSearchTerm(dp, searchTerm) {
+  searchTraces(dp, searchTerm) {


A bit of clean up - can you:

move all search-related functions together

rename find to search

add a comment-barrier-1: "search" to mark the whole region

?

Domiii · 2022-01-23T09:08:06Z

dbux-graph-host/src/graph/controllers/ContextNodeManager.js

@@ -82,28 +66,23 @@ export default class ContextNodeManager extends HostComponentEndpoint {
  }

  // TODO: makeDebounce
-  refreshOnData = () => {
+  refreshOnData = async () => {


Does this not need a makeDebounce?

Domiii · 2022-01-23T09:10:16Z

dbux-graph-host/src/graph/controllers/GraphNode.js


    // propagate mode to sub graph
    const childMode = this.getChildMode();
    for (const child of this.owner.children) {
      if (!hasGraphNode(child)) {
-        this.logger.warn(`GraphNode owner's children contains no graphNode`);
+        this.logger.warn(`GraphNode owner's children ${child.debugTag} contains no graphNode`);


This warning message is better, but still not very helpful to a dev who does not currently work on this exact feature. Usually, you want to add a bit more context information: at least we usually want to add which function/operation/feature caused this.

MiccWan and others added 4 commits January 14, 2022 17:34

add deprecation warning

c507282

Merge branch 'master' of https://github.com/Domiii/dbux into issue#619

5ccb245

add SearchController in dbux-code

117571e

show context search result in GlobalView

9370048

MiccWan requested a review from Domiii January 19, 2022 10:08

MiccWan added 5 commits January 21, 2022 15:38

fix many Search bugs

e5a6422

- fix `SearchBar` focus race condition - fix wont search when press enter after mode switch - fix undisposed handler errors - merge search `SelectorType`s in `ContextNodeManager`

try add SearchResultNode

b4bb49c

Show search result in GlobalAnalysisView

7fcdba9

- add `DataNodeNode` - use `DataNodeNode` on `DataFlowView` - add `DataNode.applicationId` - `traceSelection.isSelected` supports nodeId

Merge branch 'issue#561' into issue#619

05b01f1

focus SearchResultNode on search

623c6a3

MiccWan changed the title ~~add SearchController in dbux-code~~ add many search feature Jan 21, 2022

Domiii reviewed Jan 23, 2022

View reviewed changes

Merge branch 'master' into issue#619

920ebe7

Domiii merged commit acd7622 into master Jan 23, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add many search feature #653

add many search feature #653

MiccWan commented Jan 19, 2022 •

edited

Loading

Domiii Jan 23, 2022

Domiii Jan 23, 2022 •

edited

Loading

MiccWan Jan 25, 2022 •

edited

Loading

Domiii Jan 23, 2022 •

edited

Loading

MiccWan Jan 25, 2022

Domiii Jan 23, 2022

MiccWan Jan 25, 2022

Domiii Jan 23, 2022

MiccWan Jan 25, 2022

Domiii Jan 23, 2022

MiccWan Jan 25, 2022

Domiii Jan 23, 2022

Domiii Jan 23, 2022

Domiii Jan 23, 2022 •

edited

Loading

add many search feature #653

add many search feature #653

Conversation

MiccWan commented Jan 19, 2022 • edited Loading

Choose a reason for hiding this comment

Domiii Jan 23, 2022 • edited Loading

Choose a reason for hiding this comment

MiccWan Jan 25, 2022 • edited Loading

Choose a reason for hiding this comment

Domiii Jan 23, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Domiii Jan 23, 2022 • edited Loading

Choose a reason for hiding this comment

MiccWan commented Jan 19, 2022 •

edited

Loading

Domiii Jan 23, 2022 •

edited

Loading

MiccWan Jan 25, 2022 •

edited

Loading

Domiii Jan 23, 2022 •

edited

Loading

Domiii Jan 23, 2022 •

edited

Loading