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.

Sign up for GitHub

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

Jump to bottom

feat(core/managed): show failed, pending bubbles for any constraint type #8611

Merged
merged 3 commits into from
Oct 1, 2020
Merged

feat(core/managed): show failed, pending bubbles for any constraint type #8611

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
35d4108
feat(core/managed): show failed, pending bubbles for any constraint type
Oct 1, 2020
46775bc
fix(core/managed): get ColumnHeaders above their column content
Oct 1, 2020
7ee73e2
Merge branch 'master' into md-env-failed-statuses
mergify[bot] Oct 1, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
31 changes: 24 additions & 7 deletions app/scripts/modules/core/src/managed/ArtifactsList.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ import {
} from '../domain/IManagedEntity';
import { Icon } from '../presentation';

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

import { ISelectedArtifactVersion } from './Environments';
import { Pill } from './Pill';
import { IStatusBubbleStackProps, StatusBubbleStack } from './StatusBubbleStack';
Expand Down Expand Up @@ -134,14 +136,29 @@ 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.
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the way i'm stacking these right now is:

  1. pending constraints — this feels like a 'things are actively happening and you should know about it' case
  2. failed constraints — it feels kind of yucky to potentially not see these if other things are pending/running, but I thought 1 was more important
  3. pinned state — important, but hopefully if you don't get a chance to see it here you'll see it in the overview or via our snazzy new warning card on other versions. I'm less worried about people missing this info if there are multiple status bubbles


const isConstraintPendingManualJudgement = (constraint: IStatefulConstraint) =>
constraint.type == 'manual-judgement' && constraint.status == StatefulConstraintStatus.PENDING;
const requiresManualApproval = environments.some((environment) =>
environment.statefulConstraints?.some(isConstraintPendingManualJudgement),
const pendingConstraintTypes = new Set<string>();
const failedConstraintTypes = new Set<string>();

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

if (status === StatefulConstraintStatus.PENDING) {
pendingConstraintTypes.add(type);
} else if (status === StatefulConstraintStatus.FAIL || status === StatefulConstraintStatus.OVERRIDE_FAIL) {
failedConstraintTypes.add(type);
}
}),
);
if (requiresManualApproval) {
statuses.push({ appearance: 'progress', iconName: 'manualJudgement' });
}

pendingConstraintTypes.forEach((type) => {
statuses.push({ appearance: 'progress', iconName: getConstraintIcon(type) });
});
failedConstraintTypes.forEach((type) => {
statuses.push({ appearance: 'error', iconName: getConstraintIcon(type) });
});
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The way I've setup the logic here is that we will reduce all the constraints across all the environments into a list of their unique types – so let's say there are three pending manual judgements across three environments, we'll show one single pending manual judgement bubble. If there were any failed manual judgements, we'd show one single error manual judgement bubble. So you'll see that some environments have a constraint in that state, but no context about how many. Same is true for pinning right now.

Longer term we may need to think about supporting quantities on these bubbles so we could e.g. have a pending manual judgement bubble with a little "2" to indicate that there are two environments with that constraint status.


const isPinned = environments.some(({ pinned }) => pinned);
if (isPinned) {
Expand Down
1 change: 1 addition & 0 deletions app/scripts/modules/core/src/managed/ColumnHeader.less
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
font-size: 14px;
margin-bottom: 16px;
border-radius: 2px;
z-index: var(--layer-low);

.column-header-icon {
flex: 0 0 24px;
Expand Down