Skip to content

Commit

Permalink
fix(core): properly update execution permalink on location change (#7152
Browse files Browse the repository at this point in the history
)

* fix(core): properly update execution permalink on location change

* perf(core): limit re-rendering of Execution component on state changes
  • Loading branch information
anotherchrisberry committed Jun 27, 2019
1 parent 9bd9624 commit 058007e
Show file tree
Hide file tree
Showing 5 changed files with 69 additions and 31 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -378,16 +378,16 @@ export class PipelineGraph extends React.Component<IPipelineGraphProps, IPipelin
}

private resetLinks(props: IPipelineGraphProps, node: IPipelineGraphNode): void {
const { execution, viewState } = props;
const { activeStageId, section, stageIndex } = props.viewState;
if (props.execution) {
// executions view
node.isActive = viewState.activeStageId === node.index && viewState.executionId === execution.id;
node.isActive = activeStageId === node.index;
} else {
// pipeline config view
if (node.section === 'triggers') {
node.isActive = viewState.section === node.section;
node.isActive = section === node.section;
} else {
node.isActive = viewState.stageIndex === node.index && viewState.section === 'stage';
node.isActive = stageIndex === node.index && section === 'stage';
}
}
node.isHighlighted = false;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ export interface IExecutionViewState {
activeSubStageId: number;
canConfigure: boolean;
canTriggerPipelineManually: boolean;
executionId: string;
section?: string;
stageIndex?: number;
}
Expand Down Expand Up @@ -49,7 +48,6 @@ export interface IPipelineGraphNode {
warnings?: { messages: string[] };

// Execution node parameters
executionId?: string;
executionStage?: boolean;
hasNotStarted?: boolean;
labelComponent?: React.ComponentType<{ stage: IExecutionStageSummary }>;
Expand All @@ -68,14 +66,13 @@ export class PipelineGraphService {
const node: IPipelineGraphNode = {
childLinks: [],
children: [],
executionId: execution.id,
executionStage: true,
extraLabelLines: stage.extraLabelLines ? stage.extraLabelLines(stage) : 0,
graphRowOverride: stage.graphRowOverride || 0,
hasNotStarted: stage.hasNotStarted,
id: stage.refId,
index: idx,
isActive: viewState.activeStageId === stage.index && viewState.executionId === execution.id,
isActive: viewState.activeStageId === stage.index,
isHighlighted: false,
labelComponent: stage.labelComponent,
masterStage: stage.masterStage,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,12 +1,11 @@
import * as React from 'react';
import * as ReactGA from 'react-ga';
import { clone, isEqual } from 'lodash';
import { isEqual } from 'lodash';
import { $location } from 'ngimport';
import { Subscription } from 'rxjs';
import * as classNames from 'classnames';

import { Application } from 'core/application/application.model';
import { CopyToClipboard } from 'core/utils';
import { StageExecutionDetails } from 'core/pipeline/details/StageExecutionDetails';
import { ExecutionStatus } from 'core/pipeline/status/ExecutionStatus';
import { ParametersAndArtifacts } from 'core/pipeline/status/ParametersAndArtifacts';
Expand All @@ -25,6 +24,7 @@ import { ExecutionMarker } from './ExecutionMarker';
import { PipelineGraph } from 'core/pipeline/config/graph/PipelineGraph';
import { Tooltip } from 'core/presentation/Tooltip';
import { CancelModal } from 'core/cancelModal/CancelModal';
import { ExecutionPermalink } from './ExecutionPermalink';

import './execution.less';

Expand Down Expand Up @@ -53,7 +53,7 @@ export interface IExecutionState {
runningTimeInMs: number;
}

export class Execution extends React.Component<IExecutionProps, IExecutionState> {
export class Execution extends React.PureComponent<IExecutionProps, IExecutionState> {
public static defaultProps: Partial<IExecutionProps> = {
dataSourceKey: 'executions',
cancelHelpText: 'Cancel execution',
Expand Down Expand Up @@ -89,20 +89,23 @@ export class Execution extends React.Component<IExecutionProps, IExecutionState>
};
}

private updateViewStateDetails(): void {
const { $stateParams } = ReactInjector;
private updateViewStateDetails(toParams: any, fromParams: any): void {
const executionId = this.props.execution.id;
const { viewState } = this.state;
const newViewState = clone(viewState);
newViewState.activeStageId = Number($stateParams.stage);
newViewState.activeSubStageId = Number($stateParams.subStage);
newViewState.executionId = $stateParams.executionId;
const shouldScroll =
newViewState.executionId === this.props.execution.id && newViewState.executionId !== viewState.executionId;
if (!isEqual(viewState, newViewState)) {
const shouldShowDetails = toParams.executionId === executionId;
const shouldScroll = toParams.executionId === executionId && fromParams.executionId !== executionId;
const newViewState = { ...viewState };
newViewState.activeStageId = Number(toParams.stage);
newViewState.activeSubStageId = Number(toParams.subStage);
if (this.state.showingDetails !== shouldShowDetails) {
this.setState({
viewState: newViewState,
showingDetails: this.invalidateShowingDetails(this.props, shouldScroll),
viewState: newViewState,
});
} else {
if (this.state.showingDetails && !isEqual(viewState, newViewState)) {
this.setState({ viewState: newViewState });
}
}
}

Expand Down Expand Up @@ -199,8 +202,10 @@ export class Execution extends React.Component<IExecutionProps, IExecutionState>
this.runningTime = new OrchestratedItemRunningTime(execution, (time: number) =>
this.setState({ runningTimeInMs: time }),
);
this.stateChangeSuccessSubscription = ReactInjector.stateEvents.stateChangeSuccess.subscribe(() =>
this.updateViewStateDetails(),
this.stateChangeSuccessSubscription = ReactInjector.stateEvents.stateChangeSuccess.subscribe(
({ toParams, fromParams }) => {
this.updateViewStateDetails(toParams, fromParams);
},
);
}

Expand Down Expand Up @@ -257,10 +262,6 @@ export class Execution extends React.Component<IExecutionProps, IExecutionState>
ReactGA.event({ category: 'Pipeline', action: 'Execution source clicked' });
};

private handlePermalinkClick = (): void => {
ReactGA.event({ category: 'Pipeline', action: 'Permalink clicked' });
};

private handleToggleDetails = (): void => {
ReactGA.event({ category: 'Pipeline', action: 'Execution details toggled (Details link)' });
this.toggleDetails();
Expand Down Expand Up @@ -440,10 +441,7 @@ export class Execution extends React.Component<IExecutionProps, IExecutionState>
Source
</a>
{' | '}
<a onClick={this.handlePermalinkClick} href={this.getUrl()}>
Permalink
</a>
<CopyToClipboard text={this.getUrl()} toolTip="Copy permalink to clipboard" />
<ExecutionPermalink standalone={standalone} />
</div>
</div>
</div>
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
import { CopyToClipboard } from 'core/utils';
import { ReactInjector } from 'core/reactShims';
import * as React from 'react';
import * as ReactGA from 'react-ga';

export interface IExecutionPermalinkProps {
standalone: boolean;
}

export const ExecutionPermalink = ({ standalone }: IExecutionPermalinkProps) => {
const [url, setUrl] = React.useState(location.href);

React.useEffect(() => {
const subscription = ReactInjector.stateEvents.locationChangeSuccess.subscribe(() => {
const newUrl = location.href;
if (url !== newUrl) {
if (!standalone) {
setUrl(newUrl);
} else {
setUrl(newUrl.replace('/executions', '/executions/details'));
}
}
});
return () => subscription.unsubscribe();
}, []);

const handlePermalinkClick = (): void => {
ReactGA.event({ category: 'Pipeline', action: 'Permalink clicked' });
};

return (
<>
<a onClick={handlePermalinkClick} href={url}>
Permalink
</a>
<CopyToClipboard text={url} toolTip="Copy permalink to clipboard" />
</>
);
};
4 changes: 4 additions & 0 deletions app/scripts/modules/core/src/reactShims/state.events.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ export interface IStateChange {

export class StateEvents {
public stateChangeSuccess: Subject<IStateChange> = new Subject<IStateChange>();
public locationChangeSuccess: Subject<string> = new Subject<string>();

public static $inject = ['$rootScope'];
constructor(private $rootScope: IRootScopeService) {
Expand All @@ -23,7 +24,10 @@ export class StateEvents {
) => {
this.stateChangeSuccess.next({ to, toParams, from, fromParams });
};
const onLocationChangeSuccess = (_event: IAngularEvent, newUrl: string) => this.locationChangeSuccess.next(newUrl);

this.$rootScope.$on('$stateChangeSuccess', onChangeSuccess);
this.$rootScope.$on('$locationChangeSuccess', onLocationChangeSuccess);
}
}

Expand Down

0 comments on commit 058007e

Please sign in to comment.