Skip to content

Commit c60a74e

Browse files
jacobboykinjacobboykin
committed
fix: buncha peer review feedback to enhance ui/ux
1 parent fd1c207 commit c60a74e

16 files changed

Lines changed: 599 additions & 318 deletions

‎api/v1alpha1/annotations.go‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,11 @@ const (
2727
// hold creation time that should be cleared.
2828
AnnotationKeyClearAutoPromotionHoldCreatedAt = "kargo.akuity.io/clear-auto-promotion-hold-created-at"
2929

30+
// AnnotationKeyRollback is an annotation key that can be set on a Promotion
31+
// to indicate that it was created as a rollback to a previously verified
32+
// piece of Freight.
33+
AnnotationKeyRollback = "kargo.akuity.io/rollback"
34+
3035
// AnnotationKeyRefresh is an annotation key that can be set on a resource
3136
// to trigger a refresh of the resource by the controller. The value of the
3237
// annotation is interpreted as a token, and any change to the value of the

‎hack/testing/auto-promotion-hold/apply.sh‎

Lines changed: 91 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ FREIGHT_V002=7d96255278537d99b0c35445c3da426147d990bf
1111
FREIGHT_V003=3966bf28d5d67698bfd4816aecafd66d96a4226c
1212
API_FREIGHT_V001=10e608d617ce292f14398c07bcfadedd27b1ae6c
1313
API_FREIGHT_V002=d2f95df42a8f2ed206d7f4c15c5c5888454633eb
14+
MULTI_ORIGIN_SEED_PROMOTION=multi-origin-holds.00000000000000000000000000.seed
1415

1516
SCRIPT_DIR=$(cd -- "$(dirname -- "${BASH_SOURCE[0]}")" && pwd)
1617
NOW=$(date -u +"%Y-%m-%dT%H:%M:%SZ")
@@ -49,6 +50,27 @@ kubectl delete service slow-promotion-endpoint \
4950

5051
kubectl apply -f "${SCRIPT_DIR}/resources.yaml"
5152

53+
kubectl apply -f - <<YAML
54+
apiVersion: kargo.akuity.io/v1alpha1
55+
kind: Promotion
56+
metadata:
57+
name: ${MULTI_ORIGIN_SEED_PROMOTION}
58+
namespace: ${PROJECT}
59+
spec:
60+
stage: multi-origin-holds
61+
freight: ${FREIGHT_V001}
62+
source: nonAuto
63+
steps:
64+
- uses: set-metadata
65+
config:
66+
updates:
67+
- kind: Stage
68+
name: multi-origin-holds
69+
values:
70+
manualTest: auto-promotion-hold
71+
lastStory: multi-origin-holds
72+
YAML
73+
5274
kubectl patch stage single-origin-hold \
5375
--namespace "${PROJECT}" \
5476
--subresource=status \
@@ -115,6 +137,41 @@ kubectl patch freight "${API_FREIGHT_V001}" \
115137
JSON
116138
)"
117139

140+
kubectl patch promotion "${MULTI_ORIGIN_SEED_PROMOTION}" \
141+
--namespace "${PROJECT}" \
142+
--subresource=status \
143+
--type=merge \
144+
--patch "$(cat <<JSON
145+
{
146+
"status": {
147+
"phase": "Succeeded",
148+
"startedAt": "${NOW}",
149+
"finishedAt": "${NOW}",
150+
"freight": {
151+
"name": "${FREIGHT_V001}",
152+
"origin": {"kind": "Warehouse", "name": "auto-hold"},
153+
"images": [{"repoURL": "${IMAGE_REPO}", "tag": "1.26.0"}]
154+
},
155+
"freightCollection": {
156+
"id": "multi-origin-holds-current",
157+
"items": {
158+
"${ORIGIN_KEY}": {
159+
"name": "${FREIGHT_V001}",
160+
"origin": {"kind": "Warehouse", "name": "auto-hold"},
161+
"images": [{"repoURL": "${IMAGE_REPO}", "tag": "1.26.0"}]
162+
},
163+
"${API_ORIGIN_KEY}": {
164+
"name": "${API_FREIGHT_V001}",
165+
"origin": {"kind": "Warehouse", "name": "auto-hold-api"},
166+
"images": [{"repoURL": "${API_IMAGE_REPO}", "tag": "7.2.4"}]
167+
}
168+
}
169+
}
170+
}
171+
}
172+
JSON
173+
)"
174+
118175
kubectl patch stage single-origin-hold \
119176
--namespace "${PROJECT}" \
120177
--subresource=status \
@@ -214,6 +271,40 @@ kubectl patch stage multi-origin-holds \
214271
],
215272
"freightSummary": "2/2 origins fulfilled",
216273
"health": {"status": "Healthy"},
274+
"lastPromotion": {
275+
"name": "${MULTI_ORIGIN_SEED_PROMOTION}",
276+
"finishedAt": "${NOW}",
277+
"freight": {
278+
"name": "${FREIGHT_V001}",
279+
"origin": {"kind": "Warehouse", "name": "auto-hold"},
280+
"images": [{"repoURL": "${IMAGE_REPO}", "tag": "1.26.0"}]
281+
},
282+
"status": {
283+
"phase": "Succeeded",
284+
"startedAt": "${NOW}",
285+
"finishedAt": "${NOW}",
286+
"freight": {
287+
"name": "${FREIGHT_V001}",
288+
"origin": {"kind": "Warehouse", "name": "auto-hold"},
289+
"images": [{"repoURL": "${IMAGE_REPO}", "tag": "1.26.0"}]
290+
},
291+
"freightCollection": {
292+
"id": "multi-origin-holds-current",
293+
"items": {
294+
"${ORIGIN_KEY}": {
295+
"name": "${FREIGHT_V001}",
296+
"origin": {"kind": "Warehouse", "name": "auto-hold"},
297+
"images": [{"repoURL": "${IMAGE_REPO}", "tag": "1.26.0"}]
298+
},
299+
"${API_ORIGIN_KEY}": {
300+
"name": "${API_FREIGHT_V001}",
301+
"origin": {"kind": "Warehouse", "name": "auto-hold-api"},
302+
"images": [{"repoURL": "${API_IMAGE_REPO}", "tag": "7.2.4"}]
303+
}
304+
}
305+
}
306+
}
307+
},
217308
"freightHistory": [
218309
{
219310
"id": "multi-origin-holds-current",

‎pkg/server/promote_to_stage_v1alpha1.go‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -467,6 +467,7 @@ func (s *server) createStagePromotion(
467467
}
468468
if createdHold {
469469
createdPendingHold = hold
470+
annotateRollbackPromotion(promotion)
470471
}
471472
} else if candidate != nil && candidate.Name == freight.Name {
472473
hold, held := stage.Status.GetAutoPromotionHold(freight.Origin)
@@ -553,6 +554,13 @@ func autoPromotionHoldMatchesPendingCreate(
553554
hold.Freight.Origin.Equals(&expected.Freight.Origin)
554555
}
555556

557+
func annotateRollbackPromotion(promotion *kargoapi.Promotion) {
558+
if promotion.Annotations == nil {
559+
promotion.Annotations = make(map[string]string, 1)
560+
}
561+
promotion.Annotations[kargoapi.AnnotationKeyRollback] = kargoapi.AnnotationValueTrue
562+
}
563+
556564
// expectedAutoCandidateConflict returns a conflict when a request's
557565
// stale-candidate precondition no longer matches the current candidate.
558566
func expectedAutoCandidateConflict(

‎pkg/server/promote_to_stage_v1alpha1_test.go‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1063,6 +1063,11 @@ func Test_server_promoteToStage(t *testing.T) {
10631063
require.Equal(t, testStage.Name, promos.Items[0].Spec.Stage)
10641064
require.Equal(t, testFreight.Name, promos.Items[0].Spec.Freight)
10651065
require.Equal(t, kargoapi.PromotionSourceNonAuto, promos.Items[0].Spec.Source)
1066+
require.NotContains(
1067+
t,
1068+
promos.Items[0].Annotations,
1069+
kargoapi.AnnotationKeyRollback,
1070+
)
10661071
},
10671072
},
10681073
{
@@ -1124,6 +1129,11 @@ func Test_server_promoteToStage(t *testing.T) {
11241129
err := c.List(t.Context(), promos, client.InNamespace(testProject.Name))
11251130
require.NoError(t, err)
11261131
require.Len(t, promos.Items, 1)
1132+
require.Equal(
1133+
t,
1134+
kargoapi.AnnotationValueTrue,
1135+
promos.Items[0].Annotations[kargoapi.AnnotationKeyRollback],
1136+
)
11271137

11281138
stage := &kargoapi.Stage{}
11291139
err = c.Get(
@@ -1306,6 +1316,11 @@ func Test_server_promoteToStage(t *testing.T) {
13061316
err := c.List(t.Context(), promos, client.InNamespace(testProject.Name))
13071317
require.NoError(t, err)
13081318
require.Len(t, promos.Items, 1)
1319+
require.Equal(
1320+
t,
1321+
kargoapi.AnnotationValueTrue,
1322+
promos.Items[0].Annotations[kargoapi.AnnotationKeyRollback],
1323+
)
13091324

13101325
stage := &kargoapi.Stage{}
13111326
err = c.Get(

‎ui/src/features/project/pipelines/list/columns/action-column.tsx‎

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,13 @@
1-
import { faPause, faTruckArrowRight } from '@fortawesome/free-solid-svg-icons';
1+
import { faTruckArrowRight } from '@fortawesome/free-solid-svg-icons';
22
import { FontAwesomeIcon } from '@fortawesome/react-fontawesome';
33
import { Button, Space } from 'antd';
44
import { ColumnType } from 'antd/es/table';
55

66
import { isStageControlFlow } from '@ui/features/project/pipelines/nodes/stage-meta-utils';
7-
import { AutoPromotionHoldsPopover } from '@ui/features/project/pipelines/promotion/auto-promotion-holds-popover';
87
import { Stage } from '@ui/gen/api/v1alpha1/generated_pb';
98

9+
import { ResumeAutoPromotionAction } from './resume-auto-promotion-action';
10+
1011
type Props = {
1112
onPromote: (stage: Stage) => void;
1213
};
@@ -19,11 +20,7 @@ export const actionColumn = (props: Props): ColumnType<Stage> => ({
1920

2021
return (
2122
<Space size={6}>
22-
<AutoPromotionHoldsPopover stage={stage} placement='bottomRight'>
23-
<Button size='small' icon={<FontAwesomeIcon icon={faPause} />}>
24-
Resume
25-
</Button>
26-
</AutoPromotionHoldsPopover>
23+
<ResumeAutoPromotionAction stage={stage} />
2724
<Button
2825
onClick={() => props.onPromote(stage)}
2926
size='small'

‎ui/src/features/project/pipelines/list/columns/resume-auto-promotion-action.tsx‎

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
import { faPlay } from '@fortawesome/free-solid-svg-icons';
2+
import { FontAwesomeIcon } from '@fortawesome/react-fontawesome';
3+
import { Button } from 'antd';
4+
import { useMemo, useState } from 'react';
5+
6+
import {
7+
autoPromotionHoldStateActive,
8+
getAutoPromotionHoldEntries
9+
} from '@ui/features/project/pipelines/promotion/auto-promotion';
10+
import { ResumeAutoPromotionDrawer } from '@ui/features/project/pipelines/promotion/resume-auto-promotion-drawer';
11+
import { Stage } from '@ui/gen/api/v1alpha1/generated_pb';
12+
13+
export const ResumeAutoPromotionAction = ({ stage }: { stage: Stage }) => {
14+
const [open, setOpen] = useState(false);
15+
const hasActiveAutoPromotionHold = useMemo(
16+
() =>
17+
getAutoPromotionHoldEntries(stage).some(
18+
(entry) => entry.hold.state === autoPromotionHoldStateActive
19+
),
20+
[stage]
21+
);
22+
23+
if (!hasActiveAutoPromotionHold) {
24+
return null;
25+
}
26+
27+
return (
28+
<>
29+
<Button size='small' icon={<FontAwesomeIcon icon={faPlay} />} onClick={() => setOpen(true)}>
30+
Resume
31+
</Button>
32+
<ResumeAutoPromotionDrawer stage={stage} open={open} onClose={() => setOpen(false)} />
33+
</>
34+
);
35+
};

‎ui/src/features/project/pipelines/nodes/promotion/use-get-promotion-dropdown-items.tsx‎

Lines changed: 31 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,20 +3,28 @@ import { faBoltLightning, faCircleNotch } from '@fortawesome/free-solid-svg-icon
33
import { FontAwesomeIcon } from '@fortawesome/react-fontawesome';
44
import { Typography } from 'antd';
55
import { ItemType } from 'antd/es/menu/interface';
6+
import { useMemo, useState } from 'react';
67
import { generatePath, useNavigate } from 'react-router-dom';
78

89
import { paths } from '@ui/config/paths';
910
import { IAction, useActionContext } from '@ui/features/project/pipelines/context/action-context';
1011
import { useDictionaryContext } from '@ui/features/project/pipelines/context/dictionary-context';
1112
import { isStageControlFlow } from '@ui/features/project/pipelines/nodes/stage-meta-utils';
1213
import { useGetUpstreamFreight } from '@ui/features/project/pipelines/nodes/use-get-upstream-freight';
13-
import { getAutoPromotionCandidateName } from '@ui/features/project/pipelines/promotion/auto-promotion';
14+
import {
15+
autoPromotionHoldStateActive,
16+
getAutoPromotionCandidateName,
17+
getAutoPromotionHoldEntries
18+
} from '@ui/features/project/pipelines/promotion/auto-promotion';
19+
import { ResumeAutoPromotionDrawer } from '@ui/features/project/pipelines/promotion/resume-auto-promotion-drawer';
1420
import { useManualApprovalModal } from '@ui/features/project/pipelines/promotion/use-manual-approval-modal';
1521
import { queryFreight } from '@ui/gen/api/service/v1alpha1/service-KargoService_connectquery';
1622
import { Stage } from '@ui/gen/api/v1alpha1/generated_pb';
1723
import { useGetStageAutoPromotionCandidates, usePromoteToStage } from '@ui/gen/api/v2/core/core';
1824

1925
export const useGetPromotionDropdownItems = (stage: Stage) => {
26+
const [resumeAutoPromotionOpen, setResumeAutoPromotionOpen] = useState(false);
27+
2028
const projectName = stage?.metadata?.namespace || '';
2129
const stageName = stage?.metadata?.name || '';
2230

@@ -30,6 +38,10 @@ export const useGetPromotionDropdownItems = (stage: Stage) => {
3038
const controlFlow = isStageControlFlow(stage);
3139

3240
const upstreamFreights = useGetUpstreamFreight(stage);
41+
const autoPromotionHoldEntries = useMemo(() => getAutoPromotionHoldEntries(stage), [stage]);
42+
const hasActiveAutoPromotionHold = autoPromotionHoldEntries.some(
43+
(entry) => entry.hold.state === autoPromotionHoldStateActive
44+
);
3345

3446
const queryFreightMutation = useConnectMutation(queryFreight);
3547
const autoPromotionCandidatesQuery = useGetStageAutoPromotionCandidates(projectName, stageName, {
@@ -158,6 +170,14 @@ export const useGetPromotionDropdownItems = (stage: Stage) => {
158170
label: 'Promote',
159171
onClick: () => actionContext?.actPromote(IAction.PROMOTE, stage)
160172
});
173+
174+
if (hasActiveAutoPromotionHold) {
175+
dropdownItems.push({
176+
key: 'resume-auto-promotion',
177+
label: 'Resume auto-promotion',
178+
onClick: () => setResumeAutoPromotionOpen(true)
179+
});
180+
}
161181
}
162182

163183
if (controlFlow || totalSubscribersToThisStage > 1) {
@@ -225,5 +245,14 @@ export const useGetPromotionDropdownItems = (stage: Stage) => {
225245
});
226246
}
227247

228-
return dropdownItems;
248+
return {
249+
dropdownItems,
250+
resumeAutoPromotionDrawer: (
251+
<ResumeAutoPromotionDrawer
252+
stage={stage}
253+
open={resumeAutoPromotionOpen}
254+
onClose={() => setResumeAutoPromotionOpen(false)}
255+
/>
256+
)
257+
};
229258
};

0 commit comments

Comments
 (0)