From d5425b45b9692ccc989cacbde469343fd9919227 Mon Sep 17 00:00:00 2001 From: Chris Berry Date: Tue, 25 Jun 2019 23:38:44 -0500 Subject: [PATCH] refactor(core): virtualize execution rendering (#7140) * refactor(core): virtualize execution rendering * useEffect instead of useCallback --- .../cloudProvider/CloudProviderRegistry.ts | 2 - .../details/SingleExecutionDetails.tsx | 4 +- .../pipeline/executions/Executions.spec.tsx | 102 ++---------------- .../src/pipeline/executions/Executions.tsx | 86 +++++---------- .../executions/execution/Execution.tsx | 32 +++++- .../executionGroup/ExecutionGroup.tsx | 68 ++++++++---- .../executionGroup/ExecutionGroups.tsx | 46 +++++--- .../pipeline/filter/ExecutionFilterModel.ts | 35 +++--- .../src/pipeline/filter/ExecutionFilters.tsx | 23 ++-- .../filter/executionFilter.service.ts | 81 +++++++------- .../pipeline/service/ExecutionsTransformer.ts | 4 +- .../service/execution.service.spec.ts | 13 +-- .../src/pipeline/service/execution.service.ts | 69 +++++------- .../core/src/utils/RenderWhenVisible.tsx | 65 +++++++++++ app/scripts/modules/core/src/utils/index.ts | 5 +- 15 files changed, 317 insertions(+), 318 deletions(-) create mode 100644 app/scripts/modules/core/src/utils/RenderWhenVisible.tsx diff --git a/app/scripts/modules/core/src/cloudProvider/CloudProviderRegistry.ts b/app/scripts/modules/core/src/cloudProvider/CloudProviderRegistry.ts index 4524f031e69..6acf57541f9 100644 --- a/app/scripts/modules/core/src/cloudProvider/CloudProviderRegistry.ts +++ b/app/scripts/modules/core/src/cloudProvider/CloudProviderRegistry.ts @@ -1,6 +1,5 @@ /* tslint:disable: no-console */ import { cloneDeep, uniq, without } from 'lodash'; -import { $log } from 'ngimport'; import { SETTINGS } from 'core/config/settings'; @@ -118,7 +117,6 @@ export class CloudProviderRegistry { }); if (notFound) { - $log.debug(`No value configured for "${key}" in provider "${cloudProvider}"`); return null; } return current; diff --git a/app/scripts/modules/core/src/pipeline/details/SingleExecutionDetails.tsx b/app/scripts/modules/core/src/pipeline/details/SingleExecutionDetails.tsx index f7cc205bfba..40adc492b47 100644 --- a/app/scripts/modules/core/src/pipeline/details/SingleExecutionDetails.tsx +++ b/app/scripts/modules/core/src/pipeline/details/SingleExecutionDetails.tsx @@ -65,9 +65,7 @@ export class SingleExecutionDetails extends React.Component< executionService.getExecution($state.params.executionId).then( execution => { - const stateExecution = this.state.execution || execution; executionService.transformExecution(app, execution); - executionService.synchronizeExecution(stateExecution, execution); if (execution.isActive && !this.executionScheduler) { this.executionScheduler = SchedulerFactory.createScheduler(5000); this.executionLoader = this.executionScheduler.subscribe(() => this.getExecution()); @@ -76,7 +74,7 @@ export class SingleExecutionDetails extends React.Component< this.executionScheduler.unsubscribe(); this.executionLoader.unsubscribe(); } - this.setState({ execution: stateExecution }); + this.setState({ execution }); }, () => { this.setState({ execution: null, stateNotFound: true }); diff --git a/app/scripts/modules/core/src/pipeline/executions/Executions.spec.tsx b/app/scripts/modules/core/src/pipeline/executions/Executions.spec.tsx index 8076b0b683e..12d20035a3d 100644 --- a/app/scripts/modules/core/src/pipeline/executions/Executions.spec.tsx +++ b/app/scripts/modules/core/src/pipeline/executions/Executions.spec.tsx @@ -1,14 +1,13 @@ import * as React from 'react'; import { ReactWrapper, mount } from 'enzyme'; import { set } from 'lodash'; -import { IScope, ITimeoutService, mock, noop } from 'angular'; +import { IScope, mock, noop } from 'angular'; import { Application } from 'core/application'; import { ApplicationModelBuilder } from 'core/application/applicationModel.builder'; import { INSIGHT_FILTER_STATE_MODEL } from 'core/insight/insightFilterState.model'; -import { REACT_MODULE, ReactInjector } from 'core/reactShims'; +import { REACT_MODULE } from 'core/reactShims'; import { OVERRIDE_REGISTRY } from 'core/overrideRegistry'; -import { ScrollToService } from 'core/utils'; import * as State from 'core/state'; import { IExecutionsProps, IExecutionsState, Executions } from './Executions'; @@ -16,7 +15,6 @@ describe('', () => { let component: ReactWrapper; let application: Application; let scope: IScope; - let $timeout: ITimeoutService; function initializeApplication(data?: any) { set(application, 'executions.activate', noop); @@ -35,10 +33,9 @@ describe('', () => { beforeEach(mock.module(INSIGHT_FILTER_STATE_MODEL, REACT_MODULE, OVERRIDE_REGISTRY)); beforeEach( - mock.inject((_$timeout_: ITimeoutService, $rootScope: IScope) => { + mock.inject(($rootScope: IScope) => { State.initialize(); scope = $rootScope.$new(); - $timeout = _$timeout_; application = ApplicationModelBuilder.createApplicationForTests( 'app', { key: 'executions', lazy: true }, @@ -47,98 +44,15 @@ describe('', () => { }), ); - it('should not set loading flag to false until executions and pipeline configs have been loaded', function() { + it('should not set loading flag to false until executions and pipeline configs have been loaded', done => { initializeApplication(); expect(component.state().loading).toBe(true); application.executions.dataUpdated(); application.pipelineConfigs.dataUpdated(); scope.$digest(); - $timeout.flush(); - expect(component.state().loading).toBe(false); - }); - - describe('auto-scrolling behavior', function() { - beforeEach(function() { - spyOn(ScrollToService, 'scrollTo'); - }); - - it('should scroll execution into view on initialization if an execution is present in state params', function() { - ReactInjector.$stateParams.executionId = 'a'; - - initializeApplication({ pipelineConfigs: [], executions: [] }); - scope.$digest(); - - expect((ScrollToService.scrollTo as any).calls.count()).toBe(1); - }); - - it('should NOT scroll execution into view on initialization if none present in state params', function() { - initializeApplication(); - scope.$digest(); - - expect((ScrollToService.scrollTo as any).calls.count()).toBe(0); - }); - - // TODO: Figure out how to test transitions - // it('should scroll execution into view on state change success if no execution id in state params', function () { - // initializeApplication(); - // scope.$digest(); - - // expect((ReactInjector.scrollToService.scrollTo as any).calls.count()).toBe(0); - - // scope.$broadcast('$stateChangeSuccess', {name: 'executions.execution'}, {executionId: 'a'}, {name: 'executions'}, {}); - // expect((ReactInjector.scrollToService.scrollTo as any).calls.count()).toBe(1); - // }); - - // it('should scroll execution into view on state change success if execution id changes', function () { - // initializeApplication(); - // scope.$digest(); - - // expect((ReactInjector.scrollToService.scrollTo as any).calls.count()).toBe(0); - - // scope.$broadcast('$stateChangeSuccess', {name: 'executions.execution'}, {executionId: 'a'}, {name: 'executions.execution'}, {executionId: 'b'}); - // expect((ReactInjector.scrollToService.scrollTo as any).calls.count()).toBe(1); - // }); - - // it('should scroll into view if no params change, because the user clicked on a link somewhere else in the page', function () { - // const params = {executionId: 'a', step: 'b', stage: 'c', details: 'd'}; - - // initializeApplication(); - // scope.$digest(); - - // expect((ReactInjector.scrollToService.scrollTo as any).calls.count()).toBe(0); - - // scope.$broadcast('$stateChangeSuccess', {name: 'executions.execution'}, params, {name: 'executions.execution'}, params); - // expect((ReactInjector.scrollToService.scrollTo as any).calls.count()).toBe(1); - // }); - - // it('should NOT scroll into view if step changes', function () { - // const toParams = {executionId: 'a', step: 'b', stage: 'c', details: 'd'}, - // fromParams = {executionId: 'a', step: 'c', stage: 'c', details: 'd'}; - - // initializeApplication(); - // scope.$digest(); - // scope.$broadcast('$stateChangeSuccess', {name: 'executions'}, toParams, {name: 'executions'}, fromParams); - // expect((ReactInjector.scrollToService.scrollTo as any).calls.count()).toBe(0); - // }); - - // it('should NOT scroll into view if stage changes', function () { - // const toParams = {executionId: 'a', step: 'b', stage: 'c', details: 'd'}, - // fromParams = {executionId: 'a', step: 'b', stage: 'e', details: 'd'}; - - // initializeApplication(); - // scope.$digest(); - // scope.$broadcast('$stateChangeSuccess', {name: 'executions'}, toParams, {name: 'executions'}, fromParams); - // expect((ReactInjector.scrollToService.scrollTo as any).calls.count()).toBe(0); - // }); - - // it('should NOT scroll into view if detail changes', function () { - // const toParams = {executionId: 'a', step: 'b', stage: 'c', details: 'd'}, - // fromParams = {executionId: 'a', step: 'b', stage: 'c', details: 'e'}; - - // initializeApplication(); - // scope.$digest(); - // scope.$broadcast('$stateChangeSuccess', {name: 'executions'}, toParams, {name: 'executions'}, fromParams); - // expect((ReactInjector.scrollToService.scrollTo as any).calls.count()).toBe(0); - // }); + setTimeout(() => { + expect(component.state().loading).toBe(false); + done(); + }, 100); }); }); diff --git a/app/scripts/modules/core/src/pipeline/executions/Executions.tsx b/app/scripts/modules/core/src/pipeline/executions/Executions.tsx index b64eaf490ac..db6101d7bd5 100644 --- a/app/scripts/modules/core/src/pipeline/executions/Executions.tsx +++ b/app/scripts/modules/core/src/pipeline/executions/Executions.tsx @@ -3,9 +3,8 @@ import { CreatePipelineButton } from 'core/pipeline/create/CreatePipelineButton' import { IScheduler } from 'core/scheduler/SchedulerFactory'; import * as React from 'react'; import * as ReactGA from 'react-ga'; -import { Transition } from '@uirouter/core'; import { get } from 'lodash'; -import { $timeout, $q } from 'ngimport'; +import { $q } from 'ngimport'; import { Subscription } from 'rxjs'; import { Application } from 'core/application'; @@ -20,7 +19,6 @@ import { ExecutionGroups } from './executionGroup/ExecutionGroups'; import { FilterTags, IFilterTag, ISortFilter } from 'core/filterModel'; import { Spinner } from 'core/widgets/spinners/Spinner'; import { ExecutionState } from 'core/state'; -import { ScrollToService } from 'core/utils'; import { IRetryablePromise } from 'core/utils/retryablePromise'; import { SchedulerFactory } from 'core/scheduler'; @@ -38,12 +36,12 @@ export interface IExecutionsState { sortFilter: ISortFilter; tags: IFilterTag[]; triggeringExecution: boolean; + reloadingForFilters: boolean; } export class Executions extends React.Component { private executionsRefreshUnsubscribe: Function; private groupsUpdatedSubscription: Subscription; - private locationChangeUnsubscribe: Function; private insightFilterStateModel = ReactInjector.insightFilterStateModel; private activeRefresher: IScheduler; @@ -59,9 +57,16 @@ export class Executions extends React.Component { + if (this.state.reloadingForFilters !== reloadingForFilters) { + this.setState({ reloadingForFilters }); + } + }; + private clearFilters = (): void => { ExecutionFilterService.clearFilters(); this.updateExecutionGroups(true); @@ -74,21 +79,32 @@ export class Executions extends React.Component { + ExecutionFilterService.updateExecutionGroups(app); + setTimeout(() => this.setReloadingForFilters(false), 50); + }); } else { ExecutionFilterService.updateExecutionGroups(app); this.groupsUpdated(); - // updateExecutionGroups is debounced by 25ms, so we need to delay setting the loading flag a bit - $timeout(() => { + setTimeout(() => { this.setState({ loading: false }); }, 50); } } private groupsUpdated(): void { - this.setState({ tags: ExecutionState.filterModel.asFilterModel.tags }); + const newTags = ExecutionState.filterModel.asFilterModel.tags; + const currentTags = this.state.tags; + const areEqual = (t1: IFilterTag, t2: IFilterTag) => + t1.key === t2.key && t1.label === t2.label && t1.value === t2.value; + const tagsChanged = + newTags.length !== currentTags.length || newTags.some(t1 => !currentTags.some(t2 => areEqual(t1, t2))); + if (tagsChanged) { + this.setState({ tags: newTags }); + } } private dataInitializationFailure(): void { @@ -162,43 +178,6 @@ export class Executions extends React.Component this.groupsUpdated()); - this.locationChangeUnsubscribe = ReactInjector.$uiRouter.transitionService.onSuccess({}, t => - this.handleTransitionSuccess(t), - ); this.executionsRefreshUnsubscribe = app.executions.onRefresh( null, @@ -240,9 +216,6 @@ export class Executions extends React.Component { this.updateExecutionGroups(); - if (ReactInjector.$stateParams.executionId) { - this.scrollIntoView(); - } const nameOrIdToStart = ReactInjector.$stateParams.startManualExecution; if (nameOrIdToStart) { const toStart = app.pipelineConfigs.data.find((p: IPipeline) => [p.id, p.name].includes(nameOrIdToStart)); @@ -262,7 +235,6 @@ export class Executions extends React.Component { const { app } = this.props; - const { filtersExpanded, loading, sortFilter, tags, triggeringExecution } = this.state; + const { filtersExpanded, loading, sortFilter, tags, triggeringExecution, reloadingForFilters } = this.state; const hasPipelines = !!(get(app, 'executions.data', []).length || get(app, 'pipelineConfigs.data', []).length); @@ -338,7 +310,7 @@ export class Executions extends React.Component - {!loading && } + {!loading && }
)} - {app.executions.reloadingForFilters && ( + {reloadingForFilters && (
)} {!loading && !hasPipelines && ( @@ -460,7 +432,7 @@ export class Executions extends React.ComponentThere was an error loading executions. We'll try again shortly.
)} - {!this.state.loading && hasPipelines && } + {!loading && hasPipelines && } diff --git a/app/scripts/modules/core/src/pipeline/executions/execution/Execution.tsx b/app/scripts/modules/core/src/pipeline/executions/execution/Execution.tsx index d07ee9e57ca..c49237dc67c 100644 --- a/app/scripts/modules/core/src/pipeline/executions/execution/Execution.tsx +++ b/app/scripts/modules/core/src/pipeline/executions/execution/Execution.tsx @@ -40,6 +40,7 @@ export interface IExecutionProps { onRerun?: (execution: IExecution, config: IPipeline) => void; cancelHelpText?: string; cancelConfirmationText?: string; + scrollIntoView?: boolean; // should really only be set to ensure scrolling on initial page load deep link } export interface IExecutionState { @@ -60,6 +61,7 @@ export class Execution extends React.Component private stateChangeSuccessSubscription: Subscription; private runningTime: OrchestratedItemRunningTime; + private wrapperRef = React.createRef(); constructor(props: IExecutionProps) { super(props); @@ -77,7 +79,7 @@ export class Execution extends React.Component const restartedStage = execution.stages.find(stage => stage.context.restartDetails !== undefined); this.state = { - showingDetails: this.invalidateShowingDetails(props), + showingDetails: this.invalidateShowingDetails(props, true), showingParams: standalone, pipelinesUrl: [SETTINGS.gateUrl, 'pipelines/'].join('/'), viewState: initialViewState, @@ -94,25 +96,30 @@ export class Execution extends React.Component 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)) { this.setState({ viewState: newViewState, - showingDetails: this.invalidateShowingDetails(), + showingDetails: this.invalidateShowingDetails(this.props, shouldScroll), }); } } - private invalidateShowingDetails(props = this.props): boolean { + private invalidateShowingDetails(props = this.props, forceScroll = false): boolean { const { $state, $stateParams, executionService } = ReactInjector; const { execution, application, standalone } = props; const showing = standalone === true || (execution.id === $stateParams.executionId && $state.includes('**.execution.**')); if (showing && !execution.hydrated) { executionService.hydrate(application, execution).then(() => { - this.setState({ showingDetails: true }); + this.setState({ showingDetails: true }, () => this.scrollIntoView(forceScroll)); }); return false; } + if (forceScroll) { + this.scrollIntoView(true); + } return showing; } @@ -259,6 +266,21 @@ export class Execution extends React.Component this.toggleDetails(); }; + private scrollIntoView = (forceScroll = false) => { + const element = this.wrapperRef.current; + const { scrollIntoView, execution } = this.props; + // use a timeout to let Angular render the execution details before scrolling it into view + (scrollIntoView || forceScroll) && + element && + execution.hydrated && + setTimeout(() => { + element.scrollIntoView(); + // nudge it back down to accommodate the group header + const parent = element.closest('.all-execution-groups'); + parent && (parent.scrollTop -= 45); + }); + }; + public render() { const { application, @@ -299,7 +321,7 @@ export class Execution extends React.Component }); return ( -
+
{(title || showExecutionName) && (

diff --git a/app/scripts/modules/core/src/pipeline/executions/executionGroup/ExecutionGroup.tsx b/app/scripts/modules/core/src/pipeline/executions/executionGroup/ExecutionGroup.tsx index 11b16027c8b..29793961b21 100644 --- a/app/scripts/modules/core/src/pipeline/executions/executionGroup/ExecutionGroup.tsx +++ b/app/scripts/modules/core/src/pipeline/executions/executionGroup/ExecutionGroup.tsx @@ -2,7 +2,7 @@ import * as React from 'react'; import * as ReactGA from 'react-ga'; import { IPromise } from 'angular'; import { Subscription } from 'rxjs'; -import { find, flatten, uniq, without } from 'lodash'; +import { flatten, uniq, without } from 'lodash'; import { Application } from 'core/application/application.model'; import { CollapsibleSectionStateCache } from 'core/cache'; @@ -14,6 +14,7 @@ import { NextRunTag } from 'core/pipeline/triggers/NextRunTag'; import { Popover } from 'core/presentation/Popover'; import { ExecutionState } from 'core/state'; import { IRetryablePromise } from 'core/utils/retryablePromise'; +import { RenderWhenVisible } from 'core/utils/RenderWhenVisible'; import { TriggersTag } from 'core/pipeline/triggers/TriggersTag'; import { AccountTag } from 'core/account'; @@ -28,6 +29,7 @@ const ACCOUNT_TAG_OVERFLOW_LIMIT = 2; export interface IExecutionGroupProps { group: IExecutionGroup; application: Application; + parent: HTMLDivElement; } export interface IExecutionGroupState { @@ -35,35 +37,34 @@ export interface IExecutionGroupState { pipelineConfig: IPipeline; triggeringExecution: boolean; open: boolean; + showingDetails: boolean; poll: IRetryablePromise; displayExecutionActions: boolean; showAccounts: boolean; showOverflowAccountTags: boolean; } -export class ExecutionGroup extends React.Component { +export class ExecutionGroup extends React.PureComponent { public state: IExecutionGroupState; private expandUpdatedSubscription: Subscription; private stateChangeSuccessSubscription: Subscription; constructor(props: IExecutionGroupProps) { super(props); + const { group, application } = props; + const strategyConfig = application.strategyConfigs.data.find((c: IPipeline) => c.name === group.heading); - const strategyConfig = find(this.props.application.strategyConfigs.data, { - name: this.props.group.heading, - }) as IPipeline; - - const pipelineConfig = find(this.props.application.pipelineConfigs.data, { - name: this.props.group.heading, - }) as IPipeline; + const pipelineConfig = application.pipelineConfigs.data.find((c: IPipeline) => c.name === group.heading); const sectionCacheKey = this.getSectionCacheKey(); + const showingDetails = this.isShowingDetails(); this.state = { deploymentAccounts: this.getDeploymentAccounts(), triggeringExecution: false, + showingDetails, open: - this.isShowingDetails() || + showingDetails || !CollapsibleSectionStateCache.isSet(sectionCacheKey) || CollapsibleSectionStateCache.isExpanded(sectionCacheKey), poll: null, @@ -151,9 +152,13 @@ export class ExecutionGroup extends React.Component { // If the heading is collapsed, but we've clicked on a link to an execution in this group, expand the group + const showingDetails = this.isShowingDetails(); if (this.isShowingDetails() && !this.state.open) { this.toggle(); } + if (showingDetails !== this.state.showingDetails) { + this.setState({ showingDetails }); + } }); } @@ -201,7 +206,7 @@ export class ExecutionGroup extends React.Component { const { group } = this.props; - const { displayExecutionActions, pipelineConfig, triggeringExecution } = this.state; + const { displayExecutionActions, pipelineConfig, triggeringExecution, showingDetails } = this.state; const pipelineDisabled = pipelineConfig && pipelineConfig.disabled; const pipelineDescription = pipelineConfig && pipelineConfig.description; const hasRunningExecutions = group.runningExecutions && group.runningExecutions.length > 0; @@ -220,7 +225,7 @@ export class ExecutionGroup extends React.Component ACCOUNT_TAG_OVERFLOW_LIMIT) { groupTargetAccountLabels.push( this.setState({ showOverflowAccountTags: true })} onMouseLeave={() => this.setState({ showOverflowAccountTags: false })} > @@ -236,18 +241,9 @@ export class ExecutionGroup extends React.Component ( - - )); return ( -
+
{group.heading && (
@@ -336,7 +332,14 @@ export class ExecutionGroup extends React.ComponentNo executions found matching the selected filters.
)} - {executions} + this.renderExecutions()} + />
)} @@ -344,6 +347,25 @@ export class ExecutionGroup extends React.Component + {executions.map(execution => ( + + ))} + + ); + } + private getDeploymentAccounts(): string[] { return uniq(flatten(this.props.group.executions.map((e: IExecution) => e.deploymentTargets))) .sort() diff --git a/app/scripts/modules/core/src/pipeline/executions/executionGroup/ExecutionGroups.tsx b/app/scripts/modules/core/src/pipeline/executions/executionGroup/ExecutionGroups.tsx index a4bad0f52c6..ff0a04705ae 100644 --- a/app/scripts/modules/core/src/pipeline/executions/executionGroup/ExecutionGroups.tsx +++ b/app/scripts/modules/core/src/pipeline/executions/executionGroup/ExecutionGroups.tsx @@ -17,6 +17,7 @@ export interface IExecutionGroupsProps { export interface IExecutionGroupsState { groups: IExecutionGroup[]; showingDetails: boolean; + container?: HTMLDivElement; // need to pass the container down to children to use as root for IntersectionObserver } export class ExecutionGroups extends React.Component { @@ -28,15 +29,20 @@ export class ExecutionGroups extends React.Component { - this.forceUpdate(); + this.applicationRefreshUnsubscribe = props.application.executions.onRefresh(null, () => { + ExecutionFilterService.updateExecutionGroups(props.application); }); + this.groupsUpdatedSubscription = ExecutionFilterService.groupsUpdatedStream.subscribe(() => { - this.setState({ groups: ExecutionState.filterModel.asFilterModel.groups.slice() }); + const newGroups = ExecutionState.filterModel.asFilterModel.groups; + const { groups } = this.state; + if (newGroups.length !== groups.length || newGroups.some((g, i) => groups[i] !== g)) { + this.setState({ groups: newGroups }); + } }); this.stateChangeSuccessSubscription = stateEvents.stateChangeSuccess.subscribe(() => { const detailsShown = this.showingDetails(); @@ -47,12 +53,23 @@ export class ExecutionGroups extends React.Component g.executions.every(e => e.id !== executionId))) { + return false; + } return ReactInjector.$state.includes('**.execution'); } - public shouldComponentUpdate(_nextProps_: IExecutionGroupsProps, nextState: IExecutionGroupsState): boolean { - return nextState.groups !== this.state.groups || nextState.showingDetails !== this.state.showingDetails; - } + private setContainer = (container: HTMLDivElement) => { + if (this.state.container !== container) { + this.setState({ container }); + } + }; public componentWillUnmount(): void { if (this.applicationRefreshUnsubscribe) { @@ -68,10 +85,11 @@ export class ExecutionGroups extends React.Component { - const hasGroups = this.state.groups && this.state.groups.length > 0; - const className = `row pipelines executions ${this.state.showingDetails ? 'showing-details' : ''}`; - const executionGroups = (this.state.groups || []).map((group: IExecutionGroup) => ( - + const { groups = [], container, showingDetails } = this.state; + const hasGroups = groups.length > 0; + const className = `row pipelines executions ${showingDetails ? 'showing-details' : ''}`; + const executionGroups = groups.map((group: IExecutionGroup) => ( + )); return ( @@ -82,7 +100,9 @@ export class ExecutionGroups extends React.ComponentNo executions match the filters you've selected.

)} -
{executionGroups}
+
+ {container && executionGroups} +
); diff --git a/app/scripts/modules/core/src/pipeline/filter/ExecutionFilterModel.ts b/app/scripts/modules/core/src/pipeline/filter/ExecutionFilterModel.ts index 56a48854fd6..3874ee98c2a 100644 --- a/app/scripts/modules/core/src/pipeline/filter/ExecutionFilterModel.ts +++ b/app/scripts/modules/core/src/pipeline/filter/ExecutionFilterModel.ts @@ -1,4 +1,5 @@ import { Ng1StateDeclaration, StateParams } from '@uirouter/angularjs'; +import { ExecutionFilterService } from 'core/pipeline/filter/executionFilter.service'; import { extend } from 'lodash'; import { Subject } from 'rxjs'; import { $rootScope } from 'ngimport'; @@ -77,22 +78,32 @@ export class ExecutionFilterModel { $rootScope.$on( '$stateChangeSuccess', - (_event, toState: Ng1StateDeclaration, toParams: StateParams, fromState: Ng1StateDeclaration) => { - if (this.movingToExecutionsState(toState) && (!toParams.pipeline || !this.groupCount)) { + ( + _event, + toState: Ng1StateDeclaration, + toParams: StateParams, + fromState: Ng1StateDeclaration, + fromParams: StateParams, + ) => { + if (!this.movingToExecutionsState(toState)) { + return; + } + if ( + !this.isExecutionStateOrChild(fromState.name) || + !this.groupCount || + toParams.application !== fromParams.application + ) { this.mostRecentApplication = toParams.application; + this.asFilterModel.activate(); this.assignViewStateFromCache(); - } - if (this.movingToExecutionsState(toState) && this.isExecutionStateOrChild(fromState.name)) { - this.asFilterModel.applyParamsToUrl(); + ExecutionFilterService.updateExecutionGroups(); return; } - if (this.movingToExecutionsState(toState)) { - if (this.shouldRouteToSavedState(toParams, fromState)) { - this.asFilterModel.restoreState(toParams); - } - if (this.fromApplicationListState(fromState) && !this.asFilterModel.hasSavedState(toParams)) { - this.asFilterModel.clearFilters(); - } + if (this.shouldRouteToSavedState(toParams, fromState)) { + this.asFilterModel.restoreState(toParams); + } + if (this.fromApplicationListState(fromState) && !this.asFilterModel.hasSavedState(toParams)) { + this.asFilterModel.clearFilters(); } }, ); diff --git a/app/scripts/modules/core/src/pipeline/filter/ExecutionFilters.tsx b/app/scripts/modules/core/src/pipeline/filter/ExecutionFilters.tsx index 2b0411dd159..0193a238de3 100644 --- a/app/scripts/modules/core/src/pipeline/filter/ExecutionFilters.tsx +++ b/app/scripts/modules/core/src/pipeline/filter/ExecutionFilters.tsx @@ -1,7 +1,7 @@ import { IPromise } from 'angular'; import * as React from 'react'; import * as ReactGA from 'react-ga'; -import { get, isEmpty, orderBy, uniq } from 'lodash'; +import { get, isEmpty, orderBy, uniq, isEqual } from 'lodash'; import { Debounce } from 'lodash-decorators'; import * as classnames from 'classnames'; import { SortableContainer, SortableElement, SortableHandle, arrayMove, SortEnd } from 'react-sortable-hoc'; @@ -20,6 +20,7 @@ import './executionFilters.less'; export interface IExecutionFiltersProps { application: Application; + setReloadingForFilters: (reloadingForFilters: boolean) => void; } export interface IExecutionFiltersState { @@ -54,7 +55,7 @@ export class ExecutionFilters extends React.Component { - this.refreshPipelines(); + ExecutionFilterService.updateExecutionGroups(this.props.application); }); this.groupsUpdatedSubscription = ExecutionFilterService.groupsUpdatedStream.subscribe(() => this.setState({ tags: ExecutionState.filterModel.asFilterModel.tags }), @@ -87,8 +88,12 @@ export class ExecutionFilters extends React.Component { ExecutionState.filterModel.asFilterModel.applyParamsToUrl(); - this.props.application.executions.refresh(true); - this.props.application.executions.reloadingForFilters = true; + this.props.setReloadingForFilters(true); + const dataSource = this.props.application.getDataSource('executions'); + dataSource.refresh(true).then(() => { + ExecutionFilterService.updateExecutionGroups(this.props.application); + this.props.setReloadingForFilters(false); + }); }; private clearFilters = (): void => { @@ -119,8 +124,13 @@ export class ExecutionFilters extends React.Component this.fixName(execution, application)); @@ -171,13 +168,15 @@ export class ExecutionFilterService { private static addEmptyPipelines(groups: IExecutionGroup[], application: Application): void { const configs = (application.pipelineConfigs.data || []).concat(application.strategyConfigs.data || []); const sortFilter: ISortFilter = ExecutionState.filterModel.asFilterModel.sortFilter; + const groupNames: { [key: string]: any } = {}; + groups.forEach(g => (groupNames[g.heading] = true)); let toAdd = []; if (!this.isFilterable(sortFilter.pipeline) && !this.isFilterable(sortFilter.status) && !sortFilter.filter) { - toAdd = configs.filter((config: any) => !groups[config.name]); + toAdd = configs.filter((config: any) => !groupNames[config.name]); } else { toAdd = configs.filter((config: any) => { const filterMatches = (sortFilter.filter || '').toLowerCase().includes(config.name.toLowerCase()); - return !groups[config.name] && (sortFilter.pipeline[config.name] || filterMatches); + return !groupNames[config.name] && (sortFilter.pipeline[config.name] || filterMatches); }); } @@ -207,7 +206,9 @@ export class ExecutionFilterService { } private static fixName(execution: IExecution, application: Application): void { - const config: IPipeline = find(application.pipelineConfigs.data, { id: execution.pipelineConfigId }); + const config: IPipeline = application.pipelineConfigs.data.find( + (p: IPipeline) => p.id === execution.pipelineConfigId, + ); if (config) { execution.name = config.name; } @@ -221,7 +222,9 @@ export class ExecutionFilterService { }); executions.forEach((execution: IExecution) => { - const config: IPipeline = find(application.pipelineConfigs.data, { id: execution.pipelineConfigId }); + const config: IPipeline = application.pipelineConfigs.data.find( + (p: IPipeline) => p.id === execution.pipelineConfigId, + ); if (config != null && config.type === 'templatedPipeline') { execution.fromTemplate = true; } @@ -273,63 +276,53 @@ export class ExecutionFilterService { return groups; } - private static diffExecutionGroups(oldGroups: IExecutionGroup[], newGroups: IExecutionGroup[]): void { - const groupsToRemove: number[] = []; - - oldGroups.forEach((oldGroup, idx) => { - const newGroup = find(newGroups, { heading: oldGroup.heading }); - if (!newGroup) { - groupsToRemove.push(idx); - } else { - oldGroup.runningExecutions = newGroup.runningExecutions; - oldGroup.config = newGroup.config; - this.diffExecutions(oldGroup, newGroup); - } - }); - groupsToRemove.reverse().forEach(idx => { - oldGroups.splice(idx, 1); - }); + private static diffExecutionGroups(oldGroups: IExecutionGroup[], newGroups: IExecutionGroup[]): IExecutionGroup[] { + const diffedGroups: IExecutionGroup[] = []; newGroups.forEach(newGroup => { - const match = find(oldGroups, { heading: newGroup.heading }); - if (!match) { - oldGroups.push(newGroup); + const oldGroup = oldGroups.find(g => g.heading === newGroup.heading); + if (!oldGroup) { + diffedGroups.push(newGroup); + } else { + if (this.executionsAreDifferent(oldGroup, newGroup)) { + diffedGroups.push(newGroup); + } else { + diffedGroups.push(oldGroup); + } } }); oldGroups.forEach(group => group.executions.sort((a, b) => this.executionSorter(a, b))); + return diffedGroups; } - private static diffExecutions(oldGroup: IExecutionGroup, newGroup: IExecutionGroup): void { - const toRemove: number[] = []; - oldGroup.executions.forEach((execution, idx) => { - const newExecution = find(newGroup.executions, { id: execution.id }); + private static executionsAreDifferent(oldGroup: IExecutionGroup, newGroup: IExecutionGroup): boolean { + let changeDetected = false; + oldGroup.executions.forEach(execution => { + const newExecution = newGroup.executions.find(g => g.id === execution.id); if (!newExecution) { + changeDetected = true; $log.debug('execution no longer found, removing:', execution.id); - toRemove.push(idx); } else { if (execution.stringVal !== newExecution.stringVal) { + changeDetected = true; $log.debug('change detected, updating execution:', execution.id); - oldGroup.executions[idx] = newExecution; } } }); - toRemove.reverse().forEach(idx => { - oldGroup.executions.splice(idx, 1); - }); newGroup.executions.forEach(execution => { - const oldExecution = find(oldGroup.executions, { id: execution.id }); + const oldExecution = oldGroup.executions.find(g => g.id === execution.id); if (!oldExecution) { + changeDetected = true; $log.debug('new execution found, adding', execution.id); oldGroup.executions.push(execution); } }); + return changeDetected; } private static applyGroupsToModel(groups: IExecutionGroup[]): void { - this.diffExecutionGroups(ExecutionState.filterModel.asFilterModel.groups, groups); - - // sort groups in place so Angular doesn't try to update the world - ExecutionState.filterModel.asFilterModel.groups.sort((a: IExecutionGroup, b: IExecutionGroup) => - this.executionGroupSorter(a, b), + const filterModel = ExecutionState.filterModel.asFilterModel; + filterModel.groups = this.diffExecutionGroups(filterModel.groups, groups).sort( + (a: IExecutionGroup, b: IExecutionGroup) => this.executionGroupSorter(a, b), ); } diff --git a/app/scripts/modules/core/src/pipeline/service/ExecutionsTransformer.ts b/app/scripts/modules/core/src/pipeline/service/ExecutionsTransformer.ts index ed5b96ef667..2948f186517 100644 --- a/app/scripts/modules/core/src/pipeline/service/ExecutionsTransformer.ts +++ b/app/scripts/modules/core/src/pipeline/service/ExecutionsTransformer.ts @@ -24,7 +24,9 @@ export class ExecutionsTransformer { targets.push(...stageConfig.accountExtractor(stage)); } }); - execution.deploymentTargets = uniq(targets).sort(); + execution.deploymentTargets = uniq(targets) + .filter(a => !!a) + .sort(); } private static siblingStageSorter(a: IOrchestratedItem, b: IOrchestratedItem): number { diff --git a/app/scripts/modules/core/src/pipeline/service/execution.service.spec.ts b/app/scripts/modules/core/src/pipeline/service/execution.service.spec.ts index 175929657f3..d57c5108606 100644 --- a/app/scripts/modules/core/src/pipeline/service/execution.service.spec.ts +++ b/app/scripts/modules/core/src/pipeline/service/execution.service.spec.ts @@ -12,7 +12,6 @@ describe('Service: executionService', () => { let timeout: ITimeoutService; let $q: IQService; - beforeEach(() => State.initialize()); beforeEach(mock.module(EXECUTION_SERVICE, 'ui.router')); // https://docs.angularjs.org/guide/migration#migrate1.5to1.6-ng-services-$q @@ -34,6 +33,7 @@ describe('Service: executionService', () => { $httpBackend = _$httpBackend_; timeout = _$timeout_; $q = _$q_; + State.initialize(); State.ExecutionState.filterModel.asFilterModel.sortFilter.count = 3; }, ), @@ -352,7 +352,7 @@ describe('Service: executionService', () => { expect(application.executions.data).toEqual([original, newOne]); }); - it('should update mutated states in an existing execution if stringVal has changed', () => { + it('should replace an existing execution if stringVal has changed', () => { const originalStages = [ { id: 'a', status: 'COMPLETED' }, { id: 'b', status: 'RUNNING' }, @@ -385,15 +385,6 @@ describe('Service: executionService', () => { executionService.addExecutionsToApplication(application, execs); expect(application.executions.data).toEqual([updated]); - expect(application.executions.data).not.toBe([updated]); - expect(application.executions.data[0].stageSummaries[0]).toBe(originalStages[0]); - expect(application.executions.data[0].stageSummaries[0]).toEqual(updatedStages[0]); - expect(application.executions.data[0].stageSummaries[1]).toBe(originalStages[1]); - expect(application.executions.data[0].stageSummaries[1]).toEqual(updatedStages[1]); - expect(application.executions.data[0].stageSummaries[2]).toBe(originalStages[2]); - expect(application.executions.data[0].stageSummaries[2]).toEqual(updatedStages[2]); - expect(application.executions.data[0].stageSummaries[3]).toBe(originalStages[3]); - expect(application.executions.data[0].stageSummaries[3]).toEqual(updatedStages[3]); }); it('should replace an existing execution if status changes', () => { diff --git a/app/scripts/modules/core/src/pipeline/service/execution.service.ts b/app/scripts/modules/core/src/pipeline/service/execution.service.ts index 31d20499c5b..ad8ed94849f 100644 --- a/app/scripts/modules/core/src/pipeline/service/execution.service.ts +++ b/app/scripts/modules/core/src/pipeline/service/execution.service.ts @@ -370,7 +370,10 @@ export class ExecutionService { if (execution.status !== match.status) { application.executions.data[idx] = match; } else { - this.synchronizeExecution(execution, match); + // don't dehydrate! + if (execution.hydrated === match.hydrated) { + application.executions.data[idx] = match; + } } } } @@ -388,7 +391,7 @@ export class ExecutionService { existingData.push(execution); } }); - return existingData; + return [...existingData]; } else { return executions; } @@ -435,32 +438,6 @@ export class ExecutionService { application.runningExecutions.dataUpdated(); } - public synchronizeExecution(current: IExecution, updated: IExecution): void { - // don't dehydrate! - if (!updated.hydrated && current.hydrated) { - return; - } - const hydrationFlagChanged = !current.hydrated && updated.hydrated; - (updated.stageSummaries || []).forEach((updatedSummary, idx) => { - const currentSummary = current.stageSummaries[idx]; - if (!currentSummary) { - current.stageSummaries.push(updatedSummary); - } else { - // if the stage is active, update it in place if it has changed to save Angular - // from removing, then re-rendering every DOM node - // also, don't dehydrate - if (updatedSummary.isActive || currentSummary.isActive || hydrationFlagChanged) { - if (this.stringify(currentSummary) !== this.stringify(updatedSummary)) { - Object.assign(currentSummary, updatedSummary); - } - } - } - }); - current.stringVal = updated.stringVal; - current.hydrated = current.hydrated || updated.hydrated; - current.graphStatusHash = this.calculateGraphStatusHash(current); - } - private calculateGraphStatusHash(execution: IExecution): string { return (execution.stageSummaries || []) .map(stage => { @@ -482,22 +459,33 @@ export class ExecutionService { dataSource.data.forEach((currentExecution: IExecution, idx: number) => { if (updatedExecution.id === currentExecution.id) { updatedExecution.stringVal = this.stringifyExecution(updatedExecution); - if (updatedExecution.status !== currentExecution.status) { + if ( + updatedExecution.status !== currentExecution.status || + currentExecution.stringVal !== updatedExecution.stringVal + ) { this.transformExecution(application, updatedExecution); + updatedExecution.graphStatusHash = this.calculateGraphStatusHash(updatedExecution); dataSource.data[idx] = updatedExecution; dataSource.dataUpdated(); - } else { - if (currentExecution.stringVal !== updatedExecution.stringVal) { - this.transformExecution(application, updatedExecution); - this.synchronizeExecution(currentExecution, updatedExecution); - dataSource.dataUpdated(); - } } } }); } } + /** + * Fetches a fully hydrated execution, then assigns all its values to the supplied execution. + * If the execution is already hydrated, the operation does not re-fetch the execution. + * + * If this method is called multiple times, only the first call performs the fetch; + * subsequent calls will return the promise produced by the first call. + * + * This is a mutating operation. + * @param application the application owning the execution; needed because the stupid + * transformExecution requires it. + * @param unhydrated the execution to hydrate (which may already be hydrated) + * @return a Promise, which resolves with the execution itself. + */ public hydrate(application: Application, unhydrated: IExecution): Promise { if (unhydrated.hydrator) { return unhydrated.hydrator; @@ -507,15 +495,8 @@ export class ExecutionService { } const executionHydrator = this.getExecution(unhydrated.id).then(hydrated => { this.transformExecution(application, hydrated); - hydrated.stages.forEach(s => { - const toHydrate = unhydrated.stages.find(s2 => s2.id === s.id); - if (toHydrate) { - toHydrate.context = s.context; - toHydrate.outputs = s.outputs; - toHydrate.tasks = s.tasks; - } - }); - this.synchronizeExecution(unhydrated, hydrated); + Object.assign(unhydrated, hydrated); + unhydrated.graphStatusHash = this.calculateGraphStatusHash(unhydrated); return unhydrated; }); unhydrated.hydrator = Promise.resolve(executionHydrator); diff --git a/app/scripts/modules/core/src/utils/RenderWhenVisible.tsx b/app/scripts/modules/core/src/utils/RenderWhenVisible.tsx new file mode 100644 index 00000000000..0b4d9a5c42e --- /dev/null +++ b/app/scripts/modules/core/src/utils/RenderWhenVisible.tsx @@ -0,0 +1,65 @@ +import * as React from 'react'; + +export interface IRenderWhenVisibleProps { + // not valid without a container in px (IntersectionObserver doesn't do so good with null root but non-null rootMargin) + bufferHeight?: number; + // best guess to height of non-rendered content (in px) + placeholderHeight: number; + // set to false to call render eagerly + initiallyVisible?: boolean; + // standard render method; receives no props + render: () => JSX.Element; + // optional HTML element to use as the root in the IntersectionObserver + container?: Element; + // turns off de-rendering content + disableHide?: boolean; +} + +export const RenderWhenVisible = ({ + bufferHeight = 0, + initiallyVisible, + placeholderHeight, + render, + container, + disableHide, +}: IRenderWhenVisibleProps) => { + const [isVisible, setIsVisible] = React.useState(!!initiallyVisible); + const [height, setHeight] = React.useState(placeholderHeight); + let observer: IntersectionObserver; + + const nodeRef = React.useRef(); + + React.useEffect(() => { + let visible = isVisible; + observer = new IntersectionObserver( + entries => { + const inView = entries[0].isIntersecting; + if (inView && !visible) { + visible = true; + setIsVisible(true); + } + if (!inView && visible && !disableHide) { + visible = false; + setHeight(entries[0].boundingClientRect.height); + setIsVisible(false); + } + }, + { + root: container, + threshold: [0], + rootMargin: bufferHeight && container ? `${bufferHeight}px 0px` : '0px', + }, + ); + observer.observe(nodeRef.current); + return () => { + observer.disconnect(); + }; + }, []); + + return ( +
+ {isVisible && render()} + {!isVisible &&
} +
+ ); +}; diff --git a/app/scripts/modules/core/src/utils/index.ts b/app/scripts/modules/core/src/utils/index.ts index 53932e666d1..67d18643cf2 100644 --- a/app/scripts/modules/core/src/utils/index.ts +++ b/app/scripts/modules/core/src/utils/index.ts @@ -5,9 +5,10 @@ export * from './debug'; export * from './json/JsonUtils'; export * from './noop'; export * from './q'; +export * from './renderIfFeature.component'; +export * from './RenderWhenVisible'; export * from './scrollTo/scrollTo.service'; export * from './timeFormatters'; +export * from './unicodeBase64'; export * from './uuid.service'; export * from './workerPool'; -export * from './renderIfFeature.component'; -export * from './unicodeBase64';