From 2020bf6c9d402079efa8a8908df576ae67fa604b Mon Sep 17 00:00:00 2001 From: Kevin Woo Date: Tue, 9 Apr 2019 13:59:19 -0700 Subject: [PATCH] feat(core/execution-parameters): condense parameters/artifacts and make it collapsable (#6756) --- .../modules/core/src/domain/IPipeline.ts | 2 + .../modules/core/src/help/help.contents.ts | 1 + .../pipeline/config/parameters/parameter.html | 7 + .../pipeline/config/triggers/triggers.html | 8 +- .../details/SingleExecutionDetails.tsx | 1 + .../executions/execution/Execution.tsx | 140 +++++++++++++++++- .../executions/execution/execution.less | 16 ++ .../executionGroup/ExecutionGroup.tsx | 1 + .../src/pipeline/executions/executions.less | 5 - .../src/pipeline/status/Artifact.spec.tsx | 25 ++-- .../core/src/pipeline/status/Artifact.tsx | 13 +- .../status/ExecutionParameters.spec.tsx | 111 ++++++++++++++ .../pipeline/status/ExecutionParameters.tsx | 76 ++++++++++ .../pipeline/status/ExecutionStatus.spec.tsx | 64 -------- .../src/pipeline/status/ExecutionStatus.tsx | 46 +----- .../status/ResolvedArtifactList.spec.tsx | 63 ++++++-- .../pipeline/status/ResolvedArtifactList.tsx | 53 +++++-- .../core/src/pipeline/status/artifact.less | 33 +++-- .../src/pipeline/status/artifactList.less | 37 +++++ .../pipeline/status/executionParameters.less | 36 +++++ .../src/pipeline/status/executionStatus.less | 17 ++- 21 files changed, 569 insertions(+), 186 deletions(-) create mode 100644 app/scripts/modules/core/src/pipeline/status/ExecutionParameters.spec.tsx create mode 100644 app/scripts/modules/core/src/pipeline/status/ExecutionParameters.tsx delete mode 100644 app/scripts/modules/core/src/pipeline/status/ExecutionStatus.spec.tsx create mode 100644 app/scripts/modules/core/src/pipeline/status/executionParameters.less diff --git a/app/scripts/modules/core/src/domain/IPipeline.ts b/app/scripts/modules/core/src/domain/IPipeline.ts index 187f0bf186a..f78ed7e91ea 100644 --- a/app/scripts/modules/core/src/domain/IPipeline.ts +++ b/app/scripts/modules/core/src/domain/IPipeline.ts @@ -28,6 +28,7 @@ export interface IPipeline { type: string; }; type?: string; + pinAllParameters?: boolean; } export interface IParameter { @@ -35,6 +36,7 @@ export interface IParameter { description: string; default: string; hasOptions: boolean; + pinned: boolean; options: IParameterOption[]; condition?: IParameterCondition; } diff --git a/app/scripts/modules/core/src/help/help.contents.ts b/app/scripts/modules/core/src/help/help.contents.ts index 696d3537cc1..50989f9a927 100644 --- a/app/scripts/modules/core/src/help/help.contents.ts +++ b/app/scripts/modules/core/src/help/help.contents.ts @@ -429,6 +429,7 @@ const helpContents: { [key: string]: string } = { 'pipeline.config.parameter.label': '(Optional): a label to display when users are triggering the pipeline manually', 'pipeline.config.parameter.description': `(Optional): if supplied, will be displayed to users as a tooltip when triggering the pipeline manually. You can include HTML in this field.`, + 'pipeline.config.parameter.pinned': `(Optional): if checked, this parameter will be always shown in a pipeline execution view, otherwise it'll be collapsed by default.`, 'pipeline.config.failOnFailedExpressions': `When this option is enabled, the stage will be marked as failed if it contains any failed expressions`, 'pipeline.config.roles.help': `

When the pipeline is triggered using an automated trigger, these roles will be used to decide if the pipeline has permissions to access a protected application or account.

diff --git a/app/scripts/modules/core/src/pipeline/config/parameters/parameter.html b/app/scripts/modules/core/src/pipeline/config/parameters/parameter.html index 20a037fb3e9..d16746d80cb 100644 --- a/app/scripts/modules/core/src/pipeline/config/parameters/parameter.html +++ b/app/scripts/modules/core/src/pipeline/config/parameters/parameter.html @@ -39,6 +39,13 @@ + +
+ +
+ diff --git a/app/scripts/modules/core/src/pipeline/config/triggers/triggers.html b/app/scripts/modules/core/src/pipeline/config/triggers/triggers.html index af246f2e87e..e6c6cbaa4cf 100644 --- a/app/scripts/modules/core/src/pipeline/config/triggers/triggers.html +++ b/app/scripts/modules/core/src/pipeline/config/triggers/triggers.html @@ -1,5 +1,5 @@ - +
@@ -15,6 +15,12 @@
+
+ +
diff --git a/app/scripts/modules/core/src/pipeline/details/SingleExecutionDetails.tsx b/app/scripts/modules/core/src/pipeline/details/SingleExecutionDetails.tsx index ad174a16552..38f4ba34361 100644 --- a/app/scripts/modules/core/src/pipeline/details/SingleExecutionDetails.tsx +++ b/app/scripts/modules/core/src/pipeline/details/SingleExecutionDetails.tsx @@ -194,6 +194,7 @@ export class SingleExecutionDetails extends React.Component< 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 b035d7a5db0..96f8b3b7539 100644 --- a/app/scripts/modules/core/src/pipeline/executions/execution/Execution.tsx +++ b/app/scripts/modules/core/src/pipeline/executions/execution/Execution.tsx @@ -1,16 +1,19 @@ import * as React from 'react'; import * as ReactGA from 'react-ga'; -import { clone, isEqual } from 'lodash'; +import { clone, isEqual, keyBy, truncate } from 'lodash'; import { $location } from 'ngimport'; import { Subscription } from 'rxjs'; import * as classNames from 'classnames'; +import memoizeOne from 'memoize-one'; 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 { ExecutionParameters } from 'core/pipeline/status/ExecutionParameters'; import { IExecution, IRestartDetails, IPipeline } from 'core/domain'; import { IExecutionViewState, IPipelineGraphNode } from 'core/pipeline/config/graph/pipelineGraph.service'; +import { ResolvedArtifactList } from 'core/pipeline/status/ResolvedArtifactList'; import { OrchestratedItemRunningTime } from './OrchestratedItemRunningTime'; import { SETTINGS } from 'core/config/settings'; import { AccountTag } from 'core/account'; @@ -30,6 +33,7 @@ import './execution.less'; export interface IExecutionProps { application: Application; execution: IExecution; + pipelineConfig: IPipeline; showDurations?: boolean; standalone?: boolean; title?: string | JSX.Element; @@ -42,6 +46,7 @@ export interface IExecutionProps { export interface IExecutionState { showingDetails: boolean; + showingParams: boolean; pipelinesUrl: string; viewState: IExecutionViewState; sortFilter: ISortFilter; @@ -49,6 +54,13 @@ export interface IExecutionState { runningTimeInMs: number; } +export interface IDisplayableParameter { + key: string; + value: string; + showTruncatedValue?: boolean; + valueTruncated?: string; +} + export class Execution extends React.Component { public static defaultProps: Partial = { dataSourceKey: 'executions', @@ -60,7 +72,7 @@ export class Execution extends React.Component constructor(props: IExecutionProps) { super(props); - const { execution } = this.props; + const { execution, standalone } = this.props; const { $stateParams } = ReactInjector; const initialViewState = { @@ -75,6 +87,7 @@ export class Execution extends React.Component this.state = { showingDetails: this.invalidateShowingDetails(props), + showingParams: standalone, pipelinesUrl: [SETTINGS.gateUrl, 'pipelines/'].join('/'), viewState: initialViewState, sortFilter: ExecutionState.filterModel.asFilterModel.sortFilter, @@ -124,6 +137,11 @@ export class Execution extends React.Component }); }; + public toggleParams = (): void => { + const { showingParams } = this.state; + this.setState({ showingParams: !showingParams }); + }; + public getUrl(): string { let url = $location.absUrl(); if (!this.props.standalone) { @@ -183,6 +201,55 @@ export class Execution extends React.Component }); } + private getDisplayableParameters = memoizeOne( + ( + execution: IExecution, + pipelineConfig: IPipeline, + ): { displayableParameters: IDisplayableParameter[]; pinnedDisplayableParameters: IDisplayableParameter[] } => { + // these are internal parameters that are not useful to end users + const strategyExclusions = [ + 'parentPipelineId', + 'strategy', + 'parentStageId', + 'deploymentDetails', + 'cloudProvider', + ]; + + const truncateLength = 200; + + const isParamDisplayable = (paramKey: string) => + execution.isStrategy ? !strategyExclusions.includes(paramKey) : true; + + const displayableParameters: IDisplayableParameter[] = Object.keys( + (execution.trigger && execution.trigger.parameters) || {}, + ) + .filter(isParamDisplayable) + .sort() + .map((key: string) => { + const value = JSON.stringify(execution.trigger.parameters[key]); + const showTruncatedValue = value.length > truncateLength; + let valueTruncated; + if (showTruncatedValue) { + valueTruncated = truncate(value, { length: truncateLength }); + } + return { key, value, valueTruncated, showTruncatedValue }; + }); + + let pinnedDisplayableParameters: IDisplayableParameter[] = []; + + if (pipelineConfig) { + const paramConfigIndexByName = keyBy(pipelineConfig.parameterConfig, 'name'); + const isParamPinned = (param: IDisplayableParameter): boolean => + pipelineConfig.pinAllParameters || + (paramConfigIndexByName[param.key] && paramConfigIndexByName[param.key].pinned); // an older execution's parameter might be missing from a newer pipelineConfig.parameterConfig + + pinnedDisplayableParameters = displayableParameters.filter(isParamPinned); + } + + return { displayableParameters, pinnedDisplayableParameters }; + }, + ); + public componentDidMount(): void { const { execution } = this.props; this.runningTime = new OrchestratedItemRunningTime(execution, (time: number) => @@ -250,9 +317,25 @@ export class Execution extends React.Component ReactGA.event({ category: 'Pipeline', action: 'Permalink clicked' }); }; + private handleToggleDetails = (): void => { + ReactGA.event({ category: 'Pipeline', action: 'Execution details toggled (Details link)' }); + this.toggleDetails(); + }; + public render() { - const { application, execution, showAccountLabels, showDurations, standalone, title, cancelHelpText } = this.props; - const { pipelinesUrl, restartDetails, showingDetails, sortFilter, viewState } = this.state; + const { + application, + execution, + showAccountLabels, + showDurations, + standalone, + title, + cancelHelpText, + pipelineConfig, + } = this.props; + const { pipelinesUrl, restartDetails, showingDetails, showingParams, sortFilter, viewState } = this.state; + const { trigger } = execution; + const { artifacts, resolvedExpectedArtifacts } = trigger; const accountLabels = this.props.execution.deploymentTargets.map(account => ( @@ -280,6 +363,15 @@ export class Execution extends React.Component 'show-durations': showDurations, }); + const { displayableParameters, pinnedDisplayableParameters } = this.getDisplayableParameters( + execution, + pipelineConfig, + ); + + const parametersAndArtifactsExpanded = + showingParams || + (displayableParameters.length === pinnedDisplayableParameters.length && !resolvedExpectedArtifacts.length); + return (
@@ -299,8 +391,8 @@ export class Execution extends React.Component )}
@@ -379,7 +471,45 @@ export class Execution extends React.Component )}
+ + + + + {SETTINGS.feature.artifacts && ( + + )} + + {!standalone && ( + + )}
+ {showingDetails && (
diff --git a/app/scripts/modules/core/src/pipeline/executions/execution/execution.less b/app/scripts/modules/core/src/pipeline/executions/execution/execution.less index 8dcaaaafc9a..7293c6a8fd3 100644 --- a/app/scripts/modules/core/src/pipeline/executions/execution/execution.less +++ b/app/scripts/modules/core/src/pipeline/executions/execution/execution.less @@ -1,5 +1,7 @@ @import (reference) '~core/presentation/less/imports/commonImports.less'; +@execution-parameters-icon-width: 1.4em; + .execution { .label { text-transform: uppercase; @@ -114,3 +116,17 @@ execution-details-section-nav { min-width: 50px; } } + +.execution-parameters-button, +.execution-details-button { + font-size: 0.8em; + line-height: 12px; + + span.glyphicon { + width: @execution-parameters-icon-width; + } +} + +.execution-details-button { + margin-top: 6px; +} 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 6c98943a483..fe3480fc828 100644 --- a/app/scripts/modules/core/src/pipeline/executions/executionGroup/ExecutionGroup.tsx +++ b/app/scripts/modules/core/src/pipeline/executions/executionGroup/ExecutionGroup.tsx @@ -240,6 +240,7 @@ export class ExecutionGroup extends React.Component diff --git a/app/scripts/modules/core/src/pipeline/executions/executions.less b/app/scripts/modules/core/src/pipeline/executions/executions.less index b139bf6f5c8..a2f527ee483 100644 --- a/app/scripts/modules/core/src/pipeline/executions/executions.less +++ b/app/scripts/modules/core/src/pipeline/executions/executions.less @@ -82,11 +82,6 @@ margin-top: 0; font-weight: 600; } - &.group-by-name { - .execution-bar { - padding-top: 20px; - } - } } } } diff --git a/app/scripts/modules/core/src/pipeline/status/Artifact.spec.tsx b/app/scripts/modules/core/src/pipeline/status/Artifact.spec.tsx index 366d55d521c..39975de888b 100644 --- a/app/scripts/modules/core/src/pipeline/status/Artifact.spec.tsx +++ b/app/scripts/modules/core/src/pipeline/status/Artifact.spec.tsx @@ -23,14 +23,13 @@ describe('', () => { type: ARTIFACT_TYPE, name: ARTIFACT_NAME, }; + component = shallow(); - const dl = component.find('dl'); - const dt = dl.find('dt'); - const dd = dl.find('dd'); - expect(dl.length).toEqual(1); - expect(dt.length).toEqual(1); - expect(dd.length).toEqual(1); - expect(dd.at(0).text()).toEqual(ARTIFACT_NAME); + + const artifactName = component.find('.artifact-name'); + + expect(artifactName.length).toEqual(1); + expect(artifactName.text()).toEqual(ARTIFACT_NAME); }); it('renders an artifact version if present', function() { @@ -42,13 +41,11 @@ describe('', () => { version, }; component = shallow(); - const dl = component.find('dl'); - const dt = dl.find('dt'); - const dd = dl.find('dd'); - expect(dl.length).toEqual(1); - expect(dt.length).toEqual(2); - expect(dd.length).toEqual(2); - expect(dd.at(1).text()).toEqual(version); + + const artifactVersion = component.find('.artifact-version'); + + expect(artifactVersion.length).toEqual(1); + expect(artifactVersion.text()).toEqual(` - ${version}`); }); it('includes the artifact reference in the tootip', function() { diff --git a/app/scripts/modules/core/src/pipeline/status/Artifact.tsx b/app/scripts/modules/core/src/pipeline/status/Artifact.tsx index fd68baa8fe9..03861cae498 100644 --- a/app/scripts/modules/core/src/pipeline/status/Artifact.tsx +++ b/app/scripts/modules/core/src/pipeline/status/Artifact.tsx @@ -39,7 +39,7 @@ export class Artifact extends React.Component { return (
-
+
{ArtifactIconService.getPath(type) ? ( @@ -47,14 +47,11 @@ export class Artifact extends React.Component { {type} )}
-
{name}
+
+
{name}
+ {version &&
- {version}
} +
- {version && ( -
-
Version
-
{version}
-
- )}
); diff --git a/app/scripts/modules/core/src/pipeline/status/ExecutionParameters.spec.tsx b/app/scripts/modules/core/src/pipeline/status/ExecutionParameters.spec.tsx new file mode 100644 index 00000000000..de222762d58 --- /dev/null +++ b/app/scripts/modules/core/src/pipeline/status/ExecutionParameters.spec.tsx @@ -0,0 +1,111 @@ +import * as React from 'react'; +import { ShallowWrapper, shallow } from 'enzyme'; +import { mock } from 'angular'; + +import { REACT_MODULE } from 'core/reactShims'; + +import { IExecutionParametersProps, ExecutionParameters } from './ExecutionParameters'; +import { IDisplayableParameter } from 'core/pipeline'; +import { IPipeline } from 'core/domain'; + +describe('', () => { + let component: ShallowWrapper; + + beforeEach(mock.module(REACT_MODULE)); + beforeEach(mock.inject(() => {})); // Angular is lazy. + + it(`show only pin params, but there's no pinnedDisplayableParameters should return null`, function() { + const parameters: IDisplayableParameter[] = [{ key: '1', value: 'a' }]; + + component = shallow( + , + ); + + expect(component.get(0)).toEqual(null); + }); + + it(`show only pinned parameters in 2 columns format`, function() { + const parameters: IDisplayableParameter[] = [{ key: '1', value: 'a' }, { key: '2', value: 'b' }]; + + component = shallow( + , + ); + + expect(component.find('.execution-parameters-column').length).toEqual(2); + expect(component.find('.parameter-key').length).toEqual(2); + expect(component.find('.parameter-value').length).toEqual(2); + }); + + it(`shows pinned parameters title when all parameters are pinned`, function() { + const parameters: IDisplayableParameter[] = [{ key: '1', value: 'a' }, { key: '2', value: 'b' }]; + const pipelineConfig: IPipeline = { + application: 'my-app', + id: '123-abc', + index: 0, + name: 'my-pipeline', + stages: [], + keepWaitingPipelines: false, + limitConcurrent: false, + strategy: false, + triggers: [], + parameterConfig: [], + pinAllParameters: true, + }; + + component = shallow( + , + ); + + expect(component.find('.execution-parameters-column').length).toEqual(2); + expect(component.find('.parameter-key').length).toEqual(2); + expect(component.find('.parameter-value').length).toEqual(2); + }); + + it(`show all params, but there's no displayableParameters should return null`, function() { + const parameters: IDisplayableParameter[] = [{ key: '1', value: 'a' }]; + + component = shallow( + , + ); + + expect(component.get(0)).toEqual(null); + }); + + it(`show all parameters in 2 columns format`, function() { + const parameters: IDisplayableParameter[] = [{ key: '1', value: 'a' }, { key: '2', value: 'b' }]; + + component = shallow( + , + ); + + expect(component.find('.params-title').text()).toEqual('Parameters'); + expect(component.find('.execution-parameters-column').length).toEqual(2); + expect(component.find('.parameter-key').length).toEqual(2); + expect(component.find('.parameter-value').length).toEqual(2); + }); +}); diff --git a/app/scripts/modules/core/src/pipeline/status/ExecutionParameters.tsx b/app/scripts/modules/core/src/pipeline/status/ExecutionParameters.tsx new file mode 100644 index 00000000000..9b9391ec379 --- /dev/null +++ b/app/scripts/modules/core/src/pipeline/status/ExecutionParameters.tsx @@ -0,0 +1,76 @@ +import * as React from 'react'; + +import { IDisplayableParameter } from 'core/pipeline'; +import { IPipeline } from 'core/domain'; + +import './executionStatus.less'; +import './executionParameters.less'; + +export interface IExecutionParametersProps { + shouldShowAllParams: boolean; + displayableParameters: IDisplayableParameter[]; + pinnedDisplayableParameters: IDisplayableParameter[]; + pipelineConfig: IPipeline; +} + +interface IExecutionParametersState { + toggle: boolean; +} + +export class ExecutionParameters extends React.Component { + constructor(props: IExecutionParametersProps) { + super(props); + } + + private toggleParameterTruncation(parameter: IDisplayableParameter) { + if (parameter.valueTruncated) { + return () => { + parameter.showTruncatedValue = !parameter.showTruncatedValue; + this.setState({ toggle: parameter.showTruncatedValue }); + }; + } + return null; + } + + public render() { + const { shouldShowAllParams, displayableParameters, pinnedDisplayableParameters, pipelineConfig } = this.props; + + let parameters = pinnedDisplayableParameters; + if (shouldShowAllParams || (pipelineConfig && pipelineConfig.pinAllParameters)) { + parameters = displayableParameters; + } + + if (!parameters.length) { + return null; + } + + const halfWay = Math.ceil(parameters.length / 2); + const paramsSplitIntoColumns = [parameters.slice(0, halfWay), parameters.slice(halfWay)]; + + return ( +
+
{shouldShowAllParams && 'Parameters'}
+ +
+ {paramsSplitIntoColumns.map((c, i) => ( +
+ {c.map(p => ( +
+
{p.key}:
+
+ {p.showTruncatedValue ? p.valueTruncated : p.value} + {p.showTruncatedValue ? ( + + View Full + + ) : null} +
+
+ ))} +
+ ))} +
+
+ ); + } +} diff --git a/app/scripts/modules/core/src/pipeline/status/ExecutionStatus.spec.tsx b/app/scripts/modules/core/src/pipeline/status/ExecutionStatus.spec.tsx deleted file mode 100644 index 8676debc022..00000000000 --- a/app/scripts/modules/core/src/pipeline/status/ExecutionStatus.spec.tsx +++ /dev/null @@ -1,64 +0,0 @@ -import * as React from 'react'; -import { ShallowWrapper, shallow } from 'enzyme'; -import { mock, noop } from 'angular'; - -import { REACT_MODULE } from 'core/reactShims'; - -import { IExecution } from 'core/domain'; - -import { IExecutionStatusProps, IExecutionStatusState, ExecutionStatus } from './ExecutionStatus'; - -describe('', () => { - let component: ShallowWrapper; - - beforeEach(mock.module(REACT_MODULE)); - beforeEach(mock.inject(() => {})); // Angular is lazy. - - function getNewExecutionStatus(execution: IExecution): ShallowWrapper { - return shallow( - , - ); - } - - it('adds parameters, sorted alphabetically, to vm if present on trigger', function() { - const execution: IExecution = { - trigger: { - parameters: { - a: 'b', - b: 'c', - d: 'a', - }, - }, - } as any; - component = getNewExecutionStatus(execution); - expect(component.state().parameters).toEqual([ - { key: 'a', value: '"b"' }, - { key: 'b', value: '"c"' }, - { key: 'd', value: '"a"' }, - ]); - }); - - it('does not add parameters to vm if none present in trigger', function() { - const execution: IExecution = { trigger: {} } as any; - component = getNewExecutionStatus(execution); - expect(component.state().parameters).toEqual([]); - }); - - it('excludes some parameters if the pipeline is a strategy', function() { - const execution: IExecution = { - isStrategy: true, - trigger: { - parameters: { - included: 'a', - parentPipelineId: 'b', - strategy: 'c', - parentStageId: 'd', - deploymentDetails: 'e', - cloudProvider: 'f', - }, - }, - } as any; - component = getNewExecutionStatus(execution); - expect(component.state().parameters).toEqual([{ key: 'included', value: '"a"' }]); - }); -}); diff --git a/app/scripts/modules/core/src/pipeline/status/ExecutionStatus.tsx b/app/scripts/modules/core/src/pipeline/status/ExecutionStatus.tsx index 9c975d35241..bbae7ab2a21 100644 --- a/app/scripts/modules/core/src/pipeline/status/ExecutionStatus.tsx +++ b/app/scripts/modules/core/src/pipeline/status/ExecutionStatus.tsx @@ -1,5 +1,4 @@ import * as React from 'react'; -import * as ReactGA from 'react-ga'; import { has } from 'lodash'; import { IExecution } from 'core/domain'; @@ -10,25 +9,22 @@ import { Registry } from 'core/registry'; import { relativeTime, timestamp } from 'core/utils'; import { ISortFilter } from 'core/filterModel'; import { ExecutionState } from 'core/state'; -import { SETTINGS } from 'core/config/settings'; import { buildDisplayName } from '../executionBuild/buildDisplayName.filter'; import { ExecutionBuildLink } from '../executionBuild/ExecutionBuildLink'; import { ExecutionUserStatus } from './ExecutionUserStatus'; -import { ResolvedArtifactList } from './ResolvedArtifactList'; import './executionStatus.less'; export interface IExecutionStatusProps { execution: IExecution; - toggleDetails: (stageIndex?: number) => void; showingDetails: boolean; + showingParams: boolean; standalone: boolean; } export interface IExecutionStatusState { sortFilter: ISortFilter; - parameters: Array<{ key: string; value: any }>; timestamp: string; } @@ -38,25 +34,11 @@ export class ExecutionStatus extends React.Component = []; - const { execution } = this.props; - if (execution.trigger && execution.trigger.parameters) { - parameters = Object.keys(execution.trigger.parameters) - .sort() - .filter(paramKey => (execution.isStrategy ? !strategyExclusions.includes(paramKey) : true)) - .map((paramKey: string) => { - return { key: paramKey, value: JSON.stringify(execution.trigger.parameters[paramKey]) }; - }); - } this.state = { sortFilter: ExecutionState.filterModel.asFilterModel.sortFilter, - parameters, - timestamp: relativeTime(this.props.execution.startTime), + timestamp: relativeTime(execution.startTime), }; } @@ -100,15 +82,9 @@ export class ExecutionStatus extends React.Component { - ReactGA.event({ category: 'Pipeline', action: 'Execution details toggled (Details link)' }); - this.props.toggleDetails(); - }; - public render() { - const { execution, showingDetails, standalone } = this.props; + const { execution } = this.props; const { trigger, authentication } = execution; - const { artifacts, resolvedExpectedArtifacts } = trigger; const TriggerExecutionStatus = this.getTriggerExecutionStatus(); return (
@@ -129,23 +105,7 @@ export class ExecutionStatus extends React.Component - {this.state.parameters.map(p => ( -
  • - {p.key}: {p.value} -
    • - ))} - {SETTINGS.feature.artifacts && ( - - )} - {!standalone && ( - - - Details - - )}
    ); } diff --git a/app/scripts/modules/core/src/pipeline/status/ResolvedArtifactList.spec.tsx b/app/scripts/modules/core/src/pipeline/status/ResolvedArtifactList.spec.tsx index ea149bba97e..61c867e436a 100644 --- a/app/scripts/modules/core/src/pipeline/status/ResolvedArtifactList.spec.tsx +++ b/app/scripts/modules/core/src/pipeline/status/ResolvedArtifactList.spec.tsx @@ -19,7 +19,7 @@ describe('', () => { it('renders null when null artifacts are passed in', function() { const artifacts: IArtifact[] = null; - component = shallow(); + component = shallow(); expect(component.get(0)).toEqual(null); }); @@ -27,12 +27,16 @@ describe('', () => { const artifacts: IArtifact[] = []; const resolvedExpectedArtifacts = artifacts.map(a => ({ boundArtifact: a } as IExpectedArtifact)); component = shallow( - , + , ); expect(component.get(0)).toEqual(null); }); - it('renders a list when artifacts are passed in', function() { + it('renders null when artifacts are set to not expanded', () => { const artifacts: IArtifact[] = [ { id: 'abcd', @@ -42,9 +46,40 @@ describe('', () => { ]; const resolvedExpectedArtifacts = artifacts.map(a => ({ boundArtifact: a } as IExpectedArtifact)); component = shallow( - , + , ); - expect(component.find(Artifact).length).toEqual(1); + expect(component.get(0)).toEqual(null); + }); + + it('renders two columns when columnLayoutAfter is set to 2', function() { + const artifacts: IArtifact[] = [ + { + id: 'abcd', + type: ARTIFACT_TYPE, + name: ARTIFACT_NAME, + }, + { + id: 'efgh', + type: ARTIFACT_TYPE, + name: ARTIFACT_NAME, + }, + ]; + + const resolvedExpectedArtifacts = artifacts.map(a => ({ boundArtifact: a } as IExpectedArtifact)); + component = shallow( + , + ); + + expect(component.find('.artifact-list-column').length).toEqual(2); + expect(component.find(Artifact).length).toEqual(2); }); it('does not render an artifact without a type and name', function() { @@ -55,12 +90,16 @@ describe('', () => { ]; const resolvedExpectedArtifacts = singleArtifact.map(a => ({ boundArtifact: a } as IExpectedArtifact)); component = shallow( - , + , ); expect(component.get(0)).toEqual(null); }); - it('renders an artifacts that does have a type and name', function() { + it('only renders an artifacts that has a type and name', function() { const artifacts: IArtifact[] = [ { id: 'abcd', @@ -73,7 +112,11 @@ describe('', () => { ]; const resolvedExpectedArtifacts = artifacts.map(a => ({ boundArtifact: a } as IExpectedArtifact)); component = shallow( - , + , ); expect(component.find(Artifact).length).toEqual(1); }); @@ -86,8 +129,8 @@ describe('', () => { name: ARTIFACT_NAME, }, ]; - component = shallow(); - const li = component.find('li'); + component = shallow(); + const li = component.find('.extraneous-artifacts'); expect(li.text()).toMatch(/1.*artifact.*not.*consumed/); }); }); diff --git a/app/scripts/modules/core/src/pipeline/status/ResolvedArtifactList.tsx b/app/scripts/modules/core/src/pipeline/status/ResolvedArtifactList.tsx index 3e21693191d..112a894d205 100644 --- a/app/scripts/modules/core/src/pipeline/status/ResolvedArtifactList.tsx +++ b/app/scripts/modules/core/src/pipeline/status/ResolvedArtifactList.tsx @@ -8,11 +8,16 @@ import './artifactList.less'; export interface IResolvedArtifactListProps { artifacts: IArtifact[]; resolvedExpectedArtifacts?: IExpectedArtifact[]; + showingExpandedArtifacts: boolean; } export class ResolvedArtifactList extends React.Component { + constructor(props: IResolvedArtifactListProps) { + super(props); + } + public render() { - let { artifacts, resolvedExpectedArtifacts } = this.props; + let { artifacts, resolvedExpectedArtifacts, showingExpandedArtifacts } = this.props; artifacts = artifacts || []; resolvedExpectedArtifacts = resolvedExpectedArtifacts || []; @@ -29,28 +34,44 @@ export class ResolvedArtifactList extends React.Component rea.boundArtifact) .filter(({ name, type }) => name && type); - if (decoratedArtifacts.length === 0 && decoratedExpectedArtifacts.length === 0) { + // if there's none, don't show it + if (!showingExpandedArtifacts || (decoratedArtifacts.length === 0 && decoratedExpectedArtifacts.length === 0)) { + return null; + } + + // if we're exceeding the limit, don't show it + if (!showingExpandedArtifacts) { return null; } + const halfIndex = Math.ceil(decoratedArtifacts.length / 2); + const columns = [decoratedArtifacts.slice(0, halfIndex), decoratedArtifacts.slice(halfIndex)]; + return ( -
    -
      - {decoratedExpectedArtifacts.map((artifact: IArtifact, i: number) => { - const { reference } = artifact; - const isDefault = defaultArtifactRefs.has(reference); +
      +
      Artifacts
      +
      + {columns.map((artifactSubset: IArtifact[], colNum: number) => { return ( -
    • - -
      • +
      + {artifactSubset.map((artifact: IArtifact, i: number) => { + const { reference } = artifact; + const isDefault = defaultArtifactRefs.has(reference); + return ( +
      + +
      + ); + })} +
      ); })} - {decoratedArtifacts.length > decoratedExpectedArtifacts.length && ( -
    • - {decoratedArtifacts.length - decoratedExpectedArtifacts.length} received artifacts were not consumed -
      • - )} -
    +
    + {decoratedArtifacts.length > decoratedExpectedArtifacts.length && ( +
    + {decoratedArtifacts.length - decoratedExpectedArtifacts.length} received artifacts were not consumed +
    + )}
    ); } diff --git a/app/scripts/modules/core/src/pipeline/status/artifact.less b/app/scripts/modules/core/src/pipeline/status/artifact.less index 2a9bff31ce0..a0d951c21b2 100644 --- a/app/scripts/modules/core/src/pipeline/status/artifact.less +++ b/app/scripts/modules/core/src/pipeline/status/artifact.less @@ -1,16 +1,21 @@ .artifact-details { - div { - white-space: nowrap; - text-overflow: ellipsis; - overflow: hidden; - font-size: inherit; - height: 18px; - line-height: 18px; - } + text-overflow: ellipsis; + font-size: inherit; + line-height: 18px; + word-break: break-word; dl { margin: 0; } +} + +.artifact-detail { + display: inline-flex; + padding: 0.1px; + + .artifact-icon { + padding-right: 4px; + } dd, dt { @@ -19,10 +24,14 @@ dt { font-weight: normal; - padding: 0 4px 0 0; } +} - .artifact-icon { - padding-right: 4px; - } +.artifact-name { + padding-right: 2px; + float: left; +} + +.artifact-version { + float: left; } diff --git a/app/scripts/modules/core/src/pipeline/status/artifactList.less b/app/scripts/modules/core/src/pipeline/status/artifactList.less index e59c37c2ed9..84492385d54 100644 --- a/app/scripts/modules/core/src/pipeline/status/artifactList.less +++ b/app/scripts/modules/core/src/pipeline/status/artifactList.less @@ -1,3 +1,5 @@ +@import (reference) '~core/pipeline/executions/execution/execution.less'; + .artifact-list { ul { padding: 0; @@ -8,3 +10,38 @@ margin-bottom: 4px; } } + +.resolved-artifacts { + font-size: 0.8em; + line-height: 12px; + margin-bottom: 10px; + padding-left: @execution-parameters-icon-width - 0.2em; +} + +.resolved-artifact-list { + display: inline-block; + width: 100%; + overflow-y: hidden; + + .resolved-artifact-list::after { + clear: both; + content: ''; + } +} + +.artifacts-title { + margin-top: 5px; + margin-bottom: 2px; + font-weight: 600; +} + +.artifact-list-column { + display: flex; + flex-wrap: wrap; + flex-direction: column; + align-items: flex-start; + + width: 50%; + + float: left; +} diff --git a/app/scripts/modules/core/src/pipeline/status/executionParameters.less b/app/scripts/modules/core/src/pipeline/status/executionParameters.less new file mode 100644 index 00000000000..7e80655eb45 --- /dev/null +++ b/app/scripts/modules/core/src/pipeline/status/executionParameters.less @@ -0,0 +1,36 @@ +@import (reference) '~core/pipeline/executions/execution/execution.less'; + +.execution-parameters { + font-size: 0.8em; + line-height: 12px; + overflow: scroll; + margin-bottom: 10px; + padding-left: @execution-parameters-icon-width - 0.2em; +} + +.execution-parameters-column { + width: 50%; + float: left; +} + +.params-title { + margin-top: 5px; + margin-bottom: 2px; + font-weight: 600; +} + +.an-execution-parameter { + display: table-row; +} + +.parameter-key { + display: table-cell; + color: var(--color-concrete); + padding: 1px 2px 1px 1px; +} + +.parameter-value { + display: table-cell; + padding: 1px; + word-break: break-word; +} diff --git a/app/scripts/modules/core/src/pipeline/status/executionStatus.less b/app/scripts/modules/core/src/pipeline/status/executionStatus.less index dfe707f6426..313b2b6b317 100644 --- a/app/scripts/modules/core/src/pipeline/status/executionStatus.less +++ b/app/scripts/modules/core/src/pipeline/status/executionStatus.less @@ -2,16 +2,19 @@ display: inline-block; width: 180px; overflow-y: hidden; - font-size: .8em; + font-size: 0.8em; line-height: 12px; .status { line-height: 35px; font-weight: bold; } - .execution-type, .execution-build-number { + .execution-type, + .execution-build-number { text-transform: uppercase; } - .execution-name, .execution-type, .execution-build-number { + .execution-name, + .execution-type, + .execution-build-number { font-weight: 600; } .execution-dry-run { @@ -34,7 +37,9 @@ ul.trigger-details { margin-bottom: 5px; } - h6, h5, h4 { + h6, + h5, + h4 { margin-top: 0; margin-bottom: 0; } @@ -44,8 +49,4 @@ list-style: none; } } - - .parameter-key { - color: var(--color-concrete); - } }