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

feat(core/managed): put constraints in a 'skipped' state on skipped versions #8620

Merged
merged 2 commits into from
Oct 5, 2020
Merged
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
2 changes: 2 additions & 0 deletions app/scripts/modules/core/src/domain/IManagedEntity.ts
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,8 @@ export interface IManagedArtifactVersion {
};
}

export type IManagedArtifactVersionEnvironment = IManagedArtifactSummary['versions'][0]['environments'][0];

export interface IManagedArtifactSummary {
name: string;
type: string;
Expand Down
22 changes: 12 additions & 10 deletions app/scripts/modules/core/src/managed/ArtifactDetail.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import {
IManagedArtifactVersion,
IManagedEnvironmentSummary,
IManagedResourceSummary,
IManagedArtifactVersionEnvironment,
} from '../domain';
import { Application } from '../application';
import { useEventListener, Markdown } from '../presentation';
Expand Down Expand Up @@ -66,14 +67,21 @@ type IEnvironmentCardsProps = Pick<
IArtifactDetailProps,
'application' | 'reference' | 'version' | 'allVersions' | 'resourcesByEnvironment'
> & {
environment: IManagedArtifactSummary['versions'][0]['environments'][0];
environment: IManagedArtifactVersionEnvironment;
pinnedVersion: string;
};

const EnvironmentCards = memo(
({
application,
environment: {
environment,
reference,
version: versionDetails,
allVersions,
pinnedVersion,
resourcesByEnvironment,
}: IEnvironmentCardsProps) => {
const {
name: environmentName,
state,
deployedAt,
Expand All @@ -83,13 +91,7 @@ const EnvironmentCards = memo(
vetoed,
statefulConstraints,
statelessConstraints,
},
reference,
version: versionDetails,
allVersions,
pinnedVersion,
resourcesByEnvironment,
}: IEnvironmentCardsProps) => {
} = environment;
const {
stateService: { go },
} = useRouter();
Expand Down Expand Up @@ -156,7 +158,7 @@ const EnvironmentCards = memo(
<ConstraintCard
key={constraint.type}
application={application}
environment={environmentName}
environment={environment}
reference={reference}
version={versionDetails.version}
constraint={constraint}
Expand Down
39 changes: 23 additions & 16 deletions app/scripts/modules/core/src/managed/ArtifactsList.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import {
IStatefulConstraint,
StatefulConstraintStatus,
} from '../domain/IManagedEntity';
import { Icon } from '../presentation';
import { Icon, IconNames } from '../presentation';

import { isConstraintSupported, getConstraintIcon } from './constraints/constraintRegistry';

Expand Down Expand Up @@ -167,28 +167,35 @@ function getArtifactStatuses({ environments }: IManagedArtifactVersion): Artifac
// NOTE: The order in which entries are added to `statuses` is important. The highest priority
// item must be inserted first.

const pendingConstraintTypes = new Set<string>();
const failedConstraintTypes = new Set<string>();
const pendingConstraintIcons = new Set<IconNames>();
const failedConstraintIcons = new Set<IconNames>();

environments.forEach((environment) =>
environment.statefulConstraints?.forEach(({ type, status }: IStatefulConstraint) => {
if (!isConstraintSupported(type)) {
environments.forEach((environment) => {
if (environment.state === 'skipped') {
return;
}

environment.statefulConstraints?.forEach((constraint: IStatefulConstraint) => {
if (!isConstraintSupported(constraint.type)) {
return;
}

if (status === StatefulConstraintStatus.PENDING) {
pendingConstraintTypes.add(type);
} else if (status === StatefulConstraintStatus.FAIL || status === StatefulConstraintStatus.OVERRIDE_FAIL) {
failedConstraintTypes.add(type);
if (constraint.status === StatefulConstraintStatus.PENDING) {
pendingConstraintIcons.add(getConstraintIcon(constraint));
} else if (
constraint.status === StatefulConstraintStatus.FAIL ||
constraint.status === StatefulConstraintStatus.OVERRIDE_FAIL
) {
failedConstraintIcons.add(getConstraintIcon(constraint));
}
}),
);
});
});

pendingConstraintTypes.forEach((type) => {
statuses.push({ appearance: 'progress', iconName: getConstraintIcon(type) });
pendingConstraintIcons.forEach((iconName) => {
statuses.push({ appearance: 'progress', iconName });
});
failedConstraintTypes.forEach((type) => {
statuses.push({ appearance: 'error', iconName: getConstraintIcon(type) });
failedConstraintIcons.forEach((iconName) => {
statuses.push({ appearance: 'error', iconName });
});

const isPinned = environments.some(({ pinned }) => pinned);
Expand Down
2 changes: 1 addition & 1 deletion app/scripts/modules/core/src/managed/StatusBubble.less
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@
justify-content: center;
}

.status-inactive {
.status-future {
border: 1px dotted var(--color-icon-neutral);
}

Expand Down
8 changes: 4 additions & 4 deletions app/scripts/modules/core/src/managed/StatusBubble.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ const QUANITITY_SIZES = ['small', 'medium', 'large', 'extraLarge'];

export interface IStatusBubbleProps {
iconName: IconNames;
appearance: 'inactive' | 'neutral' | 'info' | 'progress' | 'success' | 'warning' | 'error' | 'archived';
appearance: 'future' | 'neutral' | 'info' | 'progress' | 'success' | 'warning' | 'error' | 'archived';
size: 'extraSmall' | 'small' | 'medium' | 'large' | 'extraLarge';
quantity?: string | number;
}
Expand All @@ -24,7 +24,7 @@ const paddingBySize = {
} as const;

const iconColorByAppearance = {
inactive: 'neutral',
future: 'neutral',
neutral: 'neutral',
info: 'neutral',
progress: 'neutral',
Expand Down Expand Up @@ -74,9 +74,9 @@ export const StatusBubble = memo(({ appearance, iconName, size, quantity }: ISta
<animated.div className="status-bubble-content" key={key} style={props}>
<div
className={classNames(['icon-wrapper', `status-${appearance}`, { 'with-quantity': !!quantityPill }])}
// The 'inactive' status includes a 1px border, which throws off the size of the bubble.
// The 'future' status includes a 1px border, which throws off the size of the bubble.
// We need to compensate by taking a pixel off the padding.
style={{ padding: appearance === 'inactive' ? paddingBySize[size] - 1 : paddingBySize[size] }}
style={{ padding: appearance === 'future' ? paddingBySize[size] - 1 : paddingBySize[size] }}
>
<div className="icon-container">
<Icon appearance={iconColorByAppearance[appearance]} name={item} size={size} />
Expand Down
9 changes: 7 additions & 2 deletions app/scripts/modules/core/src/managed/StatusCard.less
Original file line number Diff line number Diff line change
@@ -1,9 +1,14 @@
.StatusCard {
font-size: 13px;
color: var(--color-charcoal);
color: var(--color-icon-neutral);
border-bottom: 1px dashed rgba(0, 0, 0, 0.2);

&.status-card-inactive {
&.active {
color: var(--color-charcoal);
}

// Need to override this, because the 'future' status should always appear inactive
&.status-card-future.active {
color: var(--color-icon-neutral);
}

Expand Down
6 changes: 4 additions & 2 deletions app/scripts/modules/core/src/managed/StatusCard.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,9 @@ import { RelativeTimestamp } from './RelativeTimestamp';
import './StatusCard.less';

export interface IStatusCardProps {
appearance: 'inactive' | 'neutral' | 'info' | 'progress' | 'success' | 'warning' | 'error' | 'archived';
appearance: 'future' | 'neutral' | 'info' | 'progress' | 'success' | 'warning' | 'error' | 'archived';
background?: boolean;
active?: boolean;
iconName: IconNames;
title: React.ReactNode;
timestamp?: DateTime;
Expand All @@ -22,6 +23,7 @@ export interface IStatusCardProps {
export const StatusCard = ({
appearance,
background,
active,
iconName,
title,
timestamp,
Expand All @@ -32,7 +34,7 @@ export const StatusCard = ({
className={classNames(
'StatusCard flex-container-h space-between middle wrap sp-padding-s-yaxis sp-padding-l-xaxis',
`status-card-${appearance}`,
{ 'with-background': !!background },
{ 'with-background': !!background, active: active ?? true },
)}
>
<div className="flex-container-h middle">
Expand Down
4 changes: 2 additions & 2 deletions app/scripts/modules/core/src/managed/VersionStateCard.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -26,12 +26,12 @@ interface CardAppearance {
const cardAppearanceByState: { [state: string]: CardAppearance } = {
pending: {
icon: 'artifactPending',
appearance: 'inactive',
appearance: 'future',
title: (_: CardTitleMetadata) => 'Not deployed yet',
},
skipped: {
icon: 'artifactSkipped',
appearance: 'inactive',
appearance: 'future',
title: ({ replacedByVersionName }: CardTitleMetadata) => (
<span className="sp-group-margin-xs-xaxis">
<span>Skipped</span> <span className="text-regular">—</span>{' '}
Expand Down
40 changes: 29 additions & 11 deletions app/scripts/modules/core/src/managed/constraints/ConstraintCard.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import {
StatefulConstraintStatus,
IStatelessConstraint,
IManagedApplicationEnvironmentSummary,
IManagedArtifactVersionEnvironment,
} from '../../domain';
import { Application, ApplicationDataSource } from '../../application';
import { IRequestStatus } from '../../presentation';
Expand All @@ -28,14 +29,23 @@ import './ConstraintCard.less';
const { NOT_EVALUATED, PENDING, PASS, FAIL, OVERRIDE_PASS, OVERRIDE_FAIL } = StatefulConstraintStatus;

const constraintCardAppearanceByStatus = {
[NOT_EVALUATED]: 'inactive',
[NOT_EVALUATED]: 'future',
[PENDING]: 'info',
[PASS]: 'neutral',
[FAIL]: 'error',
[OVERRIDE_PASS]: 'neutral',
[OVERRIDE_FAIL]: 'error',
} as const;

const skippedConstraintCardAppearanceByStatus = {
[NOT_EVALUATED]: 'future',
[PENDING]: 'future',
[PASS]: 'neutral',
[FAIL]: 'neutral',
[OVERRIDE_PASS]: 'neutral',
[OVERRIDE_FAIL]: 'neutral',
} as const;

const logUnsupportedConstraintError = (type: string) => {
console.error(
new Error(`Unsupported constraint type ${type} — did you check for constraint support before rendering?`),
Expand All @@ -61,19 +71,26 @@ const overrideConstraintStatus = (
return dataSource.refresh().catch(() => null);
});

const getCardAppearance = (constraint: IStatefulConstraint | IStatelessConstraint) => {
const getCardAppearance = (
constraint: IStatefulConstraint | IStatelessConstraint,
environment: IManagedArtifactVersionEnvironment,
) => {
if (isConstraintStateful(constraint)) {
const { status } = constraint as IStatefulConstraint;
return constraintCardAppearanceByStatus[status];
if (environment.state === 'skipped') {
return skippedConstraintCardAppearanceByStatus[status];
} else {
return constraintCardAppearanceByStatus[status];
}
} else {
const { currentlyPassing } = constraint as IStatelessConstraint;
return currentlyPassing ? 'neutral' : 'inactive';
return currentlyPassing ? 'neutral' : 'future';
}
};

export interface IConstraintCardProps {
application: Application;
environment: string;
environment: IManagedArtifactVersionEnvironment;
reference: string;
version: string;
constraint: IStatefulConstraint | IStatelessConstraint;
Expand All @@ -90,14 +107,15 @@ export const ConstraintCard = memo(
return null;
}

const actions = getConstraintActions(constraint);
const actions = getConstraintActions(constraint, environment);

return (
<StatusCard
appearance={getCardAppearance(constraint)}
iconName={getConstraintIcon(type)}
timestamp={getConstraintTimestamp(constraint)}
title={getConstraintSummary(constraint)}
appearance={getCardAppearance(constraint, environment)}
active={environment.state !== 'skipped'}
iconName={getConstraintIcon(constraint)}
timestamp={getConstraintTimestamp(constraint, environment)}
title={getConstraintSummary(constraint, environment)}
actions={
actions && (
<div
Expand All @@ -114,7 +132,7 @@ export const ConstraintCard = memo(
onClick={() => {
setActionStatus('PENDING');
overrideConstraintStatus(application, {
environment,
environment: environment.name,
type,
reference,
version,
Expand Down
Loading