Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix plan view not updating #1451

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ import React from 'react'
import { render } from '@testing-library/react'
import { Provider } from 'react-redux'

import { CypherFrame } from './index'
import { CypherFrame } from './CypherFrame'
import { Frame } from 'shared/modules/stream/streamDuck'
import {
BrowserRequest,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -103,8 +103,9 @@ export type CypherFrameState = {
errors?: unknown
_asciiMaxColWidth?: number
_asciiSetColWidth?: string
_planExpand?: 'EXPAND' | 'COLLAPSE'
_planExpand: PlanExpand
}
export type PlanExpand = 'EXPAND' | 'COLLAPSE'

export class CypherFrame extends Component<CypherFrameProps, CypherFrameState> {
visElement: null | {
Expand All @@ -117,7 +118,8 @@ export class CypherFrame extends Component<CypherFrameProps, CypherFrameState> {
fullscreen: false,
collapse: false,
frameHeight: 472,
hasVis: false
hasVis: false,
_planExpand: 'EXPAND'
}

changeView(view: viewTypes.FrameView): void {
Expand Down Expand Up @@ -319,49 +321,33 @@ export class CypherFrame extends Component<CypherFrameProps, CypherFrameState> {
<RelatableView updated={this.props.request.updated} result={result} />
</Display>
<Display if={this.state.openView === viewTypes.CODE} lazy>
<CodeView
{...this.state}
result={result}
request={request}
query={query}
updated={this.props.request.updated}
setParentState={this.setState.bind(this)}
/>
<CodeView result={result} request={request} query={query} />
</Display>
<Display if={this.state.openView === viewTypes.ERRORS} lazy>
<ErrorsView
{...this.state}
result={result}
updated={this.props.request.updated}
setParentState={this.setState.bind(this)}
/>
<ErrorsView result={result} updated={this.props.request.updated} />
</Display>
<Display if={this.state.openView === viewTypes.WARNINGS} lazy>
<WarningsView
{...this.state}
result={result}
updated={this.props.request.updated}
setParentState={this.setState.bind(this)}
/>
<WarningsView result={result} updated={this.props.request.updated} />
</Display>
<Display if={this.state.openView === viewTypes.PLAN} lazy>
<PlanView
{...this.state}
_planExpand={this.state._planExpand}
result={result}
updated={this.props.request.updated}
setParentState={this.setState.bind(this)}
fullscreen={this.state.fullscreen}
assignVisElement={(svgElement: any, graphElement: any) => {
this.visElement = { svgElement, graphElement, type: 'plan' }
this.setState({ hasVis: true })
}}
setPlanExpand={(_planExpand: PlanExpand) =>
this.setState({ _planExpand })
}
/>
</Display>
<Display if={this.state.openView === viewTypes.VISUALIZATION} lazy>
<VisualizationConnectedBus
{...this.state}
result={result}
updated={this.props.request.updated}
setParentState={this.setState.bind(this)}
frameHeight={this.state.frameHeight}
assignVisElement={(svgElement: any, graphElement: any) => {
this.visElement = { svgElement, graphElement, type: 'graph' }
Expand Down Expand Up @@ -398,35 +384,23 @@ export class CypherFrame extends Component<CypherFrameProps, CypherFrameState> {
/>
</Display>
<Display if={this.state.openView === viewTypes.CODE} lazy>
<CodeStatusbar
{...this.state}
result={result}
updated={this.props.request.updated}
setParentState={this.setState.bind(this)}
/>
<CodeStatusbar result={result} />
</Display>
<Display if={this.state.openView === viewTypes.ERRORS} lazy>
<ErrorsStatusbar
{...this.state}
result={result}
updated={this.props.request.updated}
setParentState={this.setState.bind(this)}
/>
<ErrorsStatusbar result={result} />
</Display>
<Display if={this.state.openView === viewTypes.WARNINGS} lazy>
<WarningsStatusbar
{...this.state}
result={result}
updated={this.props.request.updated}
setParentState={this.setState.bind(this)}
/>
</Display>
<Display if={this.state.openView === viewTypes.PLAN} lazy>
<PlanStatusbar
{...this.state}
result={result}
updated={this.props.request.updated}
setParentState={this.setState.bind(this)}
setPlanExpand={(_planExpand: PlanExpand) =>
this.setState({ _planExpand })
}
/>
</Display>
</StyledStatsBarContainer>
Expand Down
15 changes: 10 additions & 5 deletions src/browser/modules/Stream/CypherFrame/PlanView.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -21,14 +21,13 @@
import React from 'react'
import { render } from '@testing-library/react/pure'
import neo4j from 'neo4j-driver'
import { PlanView, PlanStatusbar } from './PlanView'
import { PlanView, PlanStatusbar, PlanViewProps } from './PlanView'

describe('PlanViews', () => {
describe('PlanView', () => {
test('displays plan view if it exists', () => {
// Given
const props = {
query: 'MATCH xx0',
const props: PlanViewProps = {
result: {
summary: {
plan: {
Expand All @@ -39,7 +38,12 @@ describe('PlanViews', () => {
identifiers: ['n']
}
}
}
},
updated: 0,
fullscreen: false,
setPlanExpand: () => undefined,
assignVisElement: () => undefined,
_planExpand: 'EXPAND'
}

// When
Expand Down Expand Up @@ -69,7 +73,8 @@ describe('PlanViews', () => {
dbHits: 'xx3'
}
}
}
},
setPlanExpand: () => undefined
}

// When
Expand Down
63 changes: 41 additions & 22 deletions src/browser/modules/Stream/CypherFrame/PlanView.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
* along with this program. If not, see <http://www.gnu.org/licenses/>.
*/

import React, { Component } from 'react'
import React, { Component, ReactNode } from 'react'
import { PlanSVG } from './PlanView.styled'
import { dim } from 'browser-styles/constants'
import { deepEquals, shallowEquals } from 'services/utils'
Expand All @@ -33,10 +33,19 @@ import {
import { StyledFrameTitlebarButtonSection } from 'browser/modules/Frame/styled'
import Ellipsis from 'browser-components/Ellipsis'
import queryPlan from '../../D3Visualization/lib/visualization/components/queryPlan'
import { PlanExpand } from './CypherFrame'

type PlanViewState = { extractedPlan: any }
export type PlanViewProps = {
_planExpand: PlanExpand
setPlanExpand: (p: PlanExpand) => void
result: any
updated: any
assignVisElement: (a: any, b: any) => void
fullscreen: boolean
}

type PlanViewState = any

export class PlanView extends Component<any, PlanViewState> {
export class PlanView extends Component<PlanViewProps, PlanViewState> {
el: any
plan: any
constructor(props: any) {
Expand All @@ -48,7 +57,7 @@ export class PlanView extends Component<any, PlanViewState> {

componentDidMount() {
this.extractPlan(this.props.result)
.then(() => this.props.setParentState({ _planExpand: 'EXPAND' }))
.then(() => this.props.setPlanExpand('EXPAND'))
.catch(() => {})
}

Expand Down Expand Up @@ -151,20 +160,27 @@ export class PlanView extends Component<any, PlanViewState> {
}
}

type PlanStatusbarState = any
type PlanStatusbarState = { extractedPlan: any }
type PlanStatusbarProps = {
result: any
setPlanExpand: (p: PlanExpand) => void
}

export class PlanStatusbar extends Component<any, PlanStatusbarState> {
state = {
export class PlanStatusbar extends Component<
PlanStatusbarProps,
PlanStatusbarState
> {
state: PlanStatusbarState = {
extractedPlan: null
}

componentDidMount() {
componentDidMount(): void {
if (this.props === undefined || this.props.result === undefined) return
const extractedPlan = bolt.extractPlan(this.props.result, true)
if (extractedPlan) this.setState({ extractedPlan })
}

componentDidUpdate(prevProps: any) {
componentDidUpdate(prevProps: PlanStatusbarProps): void {
if (this.props.result === undefined) return
if (
prevProps.result === undefined ||
Expand All @@ -175,15 +191,22 @@ export class PlanStatusbar extends Component<any, PlanStatusbarState> {
}
}

shouldComponentUpdate(_props: {}, state: PlanStatusbarState) {
if (this.props.result === undefined) return true
return !deepEquals(state, this.state)
shouldComponentUpdate(
nextProps: PlanStatusbarProps,
nextState: PlanStatusbarState
): boolean {
if (!this.props?.result?.summary) return true

return (
!deepEquals(nextState, this.state) ||
!deepEquals(nextProps.result?.summary, this.props.result?.summary)
)
}

render() {
const plan: any = this.state.extractedPlan
if (!plan) return null
render(): ReactNode {
const plan = this.state.extractedPlan
const { result = {} } = this.props
if (!plan || !result.summary) return null
return (
<StyledOneRowStatsBar>
<StyledLeftPartial>
Expand All @@ -204,18 +227,14 @@ export class PlanStatusbar extends Component<any, PlanStatusbarState> {
<FrameButton
title="Collapse Plan"
data-testid="planCollapseButton"
onClick={() =>
this.props.setParentState({ _planExpand: 'COLLAPSE' })
}
onClick={() => this.props.setPlanExpand('COLLAPSE')}
>
<DoubleUpIcon />
</FrameButton>
<FrameButton
data-testid="planExpandButton"
title="Expand Plan"
onClick={() =>
this.props.setParentState({ _planExpand: 'EXPAND' })
}
onClick={() => this.props.setPlanExpand('EXPAND')}
>
<DoubleDownIcon />
</FrameButton>
Expand Down
8 changes: 4 additions & 4 deletions src/browser/modules/Stream/Stream.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
import { connect } from 'react-redux'
import React, { memo, useRef, useEffect } from 'react'
import { StyledStream, Padding, AnimationContainer } from './styled'
import CypherFrame from './CypherFrame/index'
import CypherFrame from './CypherFrame/CypherFrame'
import HistoryFrame from './HistoryFrame'
import PlayFrame from './PlayFrame'
import DefaultFrame from '../Frame/DefaultFrame'
Expand Down Expand Up @@ -52,7 +52,7 @@ import { getScrollToTop } from 'shared/modules/settings/settingsDuck'
import DbsFrame from './Auth/DbsFrame'
import EditFrame from './EditFrame'

const trans = {
const nameToFrame: Record<string, React.ComponentType<any>> = {
error: ErrorFrame,
cypher: CypherFrame,
'cypher-script': CypherScriptFrame,
Expand Down Expand Up @@ -82,10 +82,10 @@ const trans = {
default: DefaultFrame
}

type FrameType = keyof typeof trans
type FrameType = keyof typeof nameToFrame

const getFrame = (type: FrameType) => {
return trans[type] || trans.default
return nameToFrame[type] || nameToFrame.default
}

type StreamProps = {
Expand Down