Skip to content

Lineage: improved node details, withLineage - #237

Merged
labkey-nicka merged 93 commits into
masterfrom
fb_rungraph_deets
May 1, 2020
Merged

Lineage: improved node details, withLineage#237
labkey-nicka merged 93 commits into
masterfrom
fb_rungraph_deets

Conversation

@labkey-nicka

@labkey-nickalabkey-nicka commented Apr 28, 2020

Copy link
Copy Markdown
Contributor

Rationale

This PR updates our client-side <LineageGraph /> and related components to better support node details from the experiment-lineage.api. Runs, Run Steps, Data Inputs & Outputs, Material Inputs & Outputs are now supported in the detail pane.

Runs provide additional details:
image

Drill down into run steps:
image

Related Pull Requests

Changes

Lineage

  • Introduces a withLineage HoC which coordinates fetching, loading, error, and loaded states for a Lineage model. This model can be utilized by any component withLineage wraps.
  • All components that make use of Lineage, the model that wraps our experiment-lineage.api, are now wrapped by the withLineage HoC. This includes:
    • <LineageGraph/>
    • <LineageGrid/>
    • <LineageSummary/>
    • <SampleTypeLineageCounts/>
  • Removes persistence of Lineage models to reactn global store. Lineage components are no longer reactn-based rendering this persistence unnecessary as it is no longer utilized. Invalidation of lineage results, via invalidateLineageResults(), is still available, but the implementation has been moved.
  • Lineage now supports prefetching the lineage node for the specified seed. This utilizes the
    recently introduced experiment-resolve.api which allows for fetching of a single node from the
    server (as opposed to the entire lineage). This "prefetch" request is made asynchronous to the lineage request and allows for resolving the seed node to occur before the entire lineage result is loaded. This is a follow on performance enhancement for 39252.
  • fetchLineage now makes all requests against experiment-lineage.api with the includeInputsAndOutputs and includeRunSteps set to true.

Lineage Graph

  • initialModel prop has been removed in favor of using the prefetching of the seed result
    (as mentioned above).
  • Adds a new component, <LineageNodeDetailFactory/>, which contains logic for determining what component to show in the right-hand side of the <LineageGraph/>. This logic was formerly handled via LineageGraph.renderSelectedNodes().

Lineage Grid

  • Factored out binding of properties from Location into a new component, <LineageGridFromLocation/>, which takes a location and parses the props into a <LineageGrid />.
  • <LineageGrid /> still derives a model from the provided Lineage, however, this now uses getDerivedStateFromProps to produce the next state.

withQueryModels

  • Adds <DetailPanelWithModel /> which utilizes withQueryModels to render "detail" information from a QueryModel.
  • A presentational component, <DetailDisplay/>, has been factored out of the QueryGridModel-based <Detail/> and is reused by he QueryModel-based <DetailPanelWithModel/>.
  • Makes LoadingState a reusable enum across components. Factored out of QueryModel and additionally used by Lineage.
  • A getter for detailColumns and updateColumns has been added to QueryInfo to allow for shared access to this logic.
  • A getter for isDetailColumn and isUpdateColumn has been added to QueryColumn to allow for shared access to this logic.

- attempt to fuzzy match against run types
- allows for quickly populating side pane
- can be enabled via "prefetchSeed" option on a lineage graph (true by default)
- common display component, DetailDisplay, shared with Detail (QueryGridModel variant)
- wraps DetailWithModel to provide QueryModel based component for lineage nodes
# Conflicts:
#	packages/components/package.json
#	packages/components/src/components/base/models/QueryInfo.ts
#	packages/components/src/components/forms/detail/Detail.tsx
#	packages/components/src/components/forms/detail/DetailEditRenderer.tsx
#	packages/components/src/components/lineage/Lineage.spec.ts
#	packages/components/src/components/lineage/LineageGraph.tsx
#	packages/components/src/components/lineage/LineageSummary.tsx
#	packages/components/src/components/lineage/SampleTypeLineageCounts.tsx
#	packages/components/src/components/lineage/actions.ts
#	packages/components/src/components/lineage/grid/LineageGrid.spec.tsx
#	packages/components/src/components/lineage/grid/LineageGrid.tsx
#	packages/components/src/components/lineage/grid/LineageGridDisplay.tsx
#	packages/components/src/components/lineage/models.ts
#	packages/components/src/components/lineage/node/LineageNodeDetail.tsx
#	packages/components/src/components/lineage/node/LineageNodeList.tsx
#	packages/components/src/components/lineage/types.ts
#	packages/components/src/components/lineage/utils.ts
#	packages/components/src/components/lineage/vis/VisGraph.tsx
#	packages/components/src/components/lineage/vis/VisGraphGenerator.ts
#	packages/components/src/index.ts
#	packages/components/src/stories/Lineage.tsx
#	packages/components/src/stories/mock.tsx
- provides "lineage" as a context provided value to sub-nodes of a withLineage wrapped component
- caches result + post-processing
- custom components for rendering run steps, node lists
- fix will be applied to server-side for dispensing decoded schemaNames
This reverts commit f5f3f27
- shared anchor wrapping component for displaying hover links in lineage
# Conflicts:
#	packages/components/package.json
#	packages/components/src/index.ts
#	packages/components/src/stories/mock.tsx

@labkey-alanlabkey-alan left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, but I did leave some specific pieces of feedback. Also worth noting VisGraph.highlightNode is unused and probably worth deleting?

Comment threadpackages/components/src/QueryModel/QueryModel.ts Outdated
Comment threadpackages/components/src/QueryModel/QueryModel.ts Outdated
Comment threadpackages/components/src/components/base/models/QueryInfo.ts Outdated
Comment threadpackages/components/src/components/lineage/SampleTypeLineageCounts.tsx Outdated
Comment threadpackages/components/src/components/lineage/actions.ts Outdated
Comment threadpackages/components/src/components/lineage/node/DetailsList.tsx Outdated
Comment threadpackages/components/src/components/lineage/node/LineageNodeDetail.tsx Outdated
Comment threadpackages/components/src/components/lineage/node/LineageNodeDetail.tsx Outdated
Comment threadpackages/components/src/components/lineage/utils.ts Outdated
Comment threadpackages/components/src/components/lineage/vis/VisGraphGenerator.ts Outdated
@labkey-nicka

Copy link
Copy Markdown
ContributorAuthor

Looks good, but I did leave some specific pieces of feedback. Also worth noting VisGraph.highlightNode is unused and probably worth deleting?

It actually is in use in <LineageGraph /> via ref. I've updated the typings for the ref so that hopefully IntelliJ will better understand what is going on.

# Conflicts:
#	packages/components/package.json
#	packages/components/src/components/lineage/LineageSummary.tsx
#	packages/components/src/components/lineage/types.ts
#	packages/components/src/index.ts
#	packages/components/src/stories/Lineage.tsx
return this.queryInfo?.getColumn(fieldKey);
}

get detailColumns(): QueryColumn[] {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should the function return type be QueryColumn[] | undefined ?

return this.queryInfo?.getAllColumns(this.viewName, List(this.omittedColumns)).toArray();
}

get updateColumns(): QueryColumn[] {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should the function return type be QueryColumn[] | undefined ?

body = (
<div>
{data.map((row: any, i: number) => {
// key safety

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The 'key safety' comment could be more descriptive. Creating a case-insensitive (ordered) map seems like something we might have a utility method for. If not, then maybe we should?

@labkey-nicka
labkey-nicka merged commit cf8ede5 into masterMay 1, 2020
@labkey-nicka
labkey-nicka deleted the fb_rungraph_deets branch May 1, 2020 15:11
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@labkey-nicka@labkey-kevink@labkey-alan