From c7802cb98c637f777a8f3c665642336affc1b5e3 Mon Sep 17 00:00:00 2001 From: Lisa Kim Date: Tue, 22 Oct 2024 22:20:51 -0700 Subject: [PATCH] Address CRs --- .../RequestCheckout/RequestCheckout.tsx | 3 +- .../AccessRequestCheckout.tsx | 203 +++++++++--------- .../useAccessRequestCheckout.test.tsx | 2 +- .../useAccessRequestCheckout.ts | 22 +- .../NewRequest/useNewRequest.ts | 9 +- 5 files changed, 117 insertions(+), 122 deletions(-) diff --git a/web/packages/shared/components/AccessRequests/NewRequest/RequestCheckout/RequestCheckout.tsx b/web/packages/shared/components/AccessRequests/NewRequest/RequestCheckout/RequestCheckout.tsx index 131f4f3c4a535..46b8e3fe0a499 100644 --- a/web/packages/shared/components/AccessRequests/NewRequest/RequestCheckout/RequestCheckout.tsx +++ b/web/packages/shared/components/AccessRequests/NewRequest/RequestCheckout/RequestCheckout.tsx @@ -47,6 +47,7 @@ import { Option } from 'shared/components/Select'; import { FieldCheckbox } from 'shared/components/FieldCheckbox'; import { mergeRefs } from 'shared/libs/mergeRefs'; import { RequestableResourceKind } from 'shared/components/AccessRequests/NewRequest/resource'; +import { TextSelectCopyMulti } from 'shared/components/TextSelectCopy'; import { CreateRequest } from '../../Shared/types'; import { AssumeStartTime } from '../../AssumeStartTime/AssumeStartTime'; @@ -66,8 +67,6 @@ import { CrossIcon } from './CrossIcon'; import type { TransitionStatus } from 'react-transition-group'; import type { AccessRequest } from 'shared/services/accessRequests'; -import { P } from 'design/Text/Text'; -import { TextSelectCopyMulti } from 'shared/components/TextSelectCopy'; export const RequestCheckoutWithSlider = forwardRef< HTMLDivElement, diff --git a/web/packages/teleterm/src/ui/AccessRequestCheckout/AccessRequestCheckout.tsx b/web/packages/teleterm/src/ui/AccessRequestCheckout/AccessRequestCheckout.tsx index 9adf9f963b9da..dbcc521911839 100644 --- a/web/packages/teleterm/src/ui/AccessRequestCheckout/AccessRequestCheckout.tsx +++ b/web/packages/teleterm/src/ui/AccessRequestCheckout/AccessRequestCheckout.tsx @@ -114,123 +114,126 @@ export function AccessRequestCheckout() { setShowCheckout(false); } - const filteredData = pendingAccessRequests.filter( - d => !isKubeClusterWithNamespaces(d, pendingAccessRequests) - ); + const pendingAccessRequestsWithoutParentResource = + pendingAccessRequests.filter( + d => !isKubeClusterWithNamespaces(d, pendingAccessRequests) + ); - const numAddedResources = filteredData.length; + const numAddedResources = pendingAccessRequestsWithoutParentResource.length; // We should rather detect how much space we have, // but for simplicity we only count items. const moreToShow = Math.max( - filteredData.length - MAX_RESOURCES_IN_BAR_TO_SHOW, + pendingAccessRequestsWithoutParentResource.length - + MAX_RESOURCES_IN_BAR_TO_SHOW, 0 ); return ( <> - {filteredData.length > 0 && !isCollapsed() && ( - props.theme.colors.spotBackground[1]}; - `} - > - 0 && + !isCollapsed() && ( + props.theme.space[1]}px; + border-top: 1px solid + ${props => props.theme.colors.spotBackground[1]}; `} > - - - {numAddedResources}{' '} - {pluralize( - numAddedResources, - isRoleRequest ? 'role' : 'resource' - )}{' '} - added to access request: - - - {filteredData - .slice(0, MAX_RESOURCES_IN_BAR_TO_SHOW) - .map(c => { - let resource = { - name: c.subResourceName - ? `${c.id}/${c.subResourceName}` - : c.name, - key: `${c.clusterName}-${c.kind}-${c.id}-${c.subResourceName}`, - Icon: undefined, - }; - switch (c.kind) { - case 'app': - case 'saml_idp_service_provider': - resource.Icon = Icon.Application; - break; - case 'node': - resource.Icon = Icon.Server; - break; - case 'db': - resource.Icon = Icon.Database; - break; - case 'kube_cluster': - case 'namespace': - resource.Icon = Icon.Kubernetes; - break; - case 'role': - break; - default: - c satisfies never; - } - return resource; - }) - .map(c => ( - - ))} - {!!moreToShow && ( - - )} + {c.Icon && } + + {c.name} + + + ))} + {!!moreToShow && ( + + )} + + + + setShowCheckout(!showCheckout)} + textTransform="none" + css={` + white-space: nowrap; + `} + > + Proceed to request + + + + - - setShowCheckout(!showCheckout)} - textTransform="none" - css={` - white-space: nowrap; - `} - > - Proceed to request - - - - - - - - )} + + )} {assumedRequests.map(request => ( ))} diff --git a/web/packages/teleterm/src/ui/AccessRequestCheckout/useAccessRequestCheckout.test.tsx b/web/packages/teleterm/src/ui/AccessRequestCheckout/useAccessRequestCheckout.test.tsx index 9530e74cbbf67..a4dc881042005 100644 --- a/web/packages/teleterm/src/ui/AccessRequestCheckout/useAccessRequestCheckout.test.tsx +++ b/web/packages/teleterm/src/ui/AccessRequestCheckout/useAccessRequestCheckout.test.tsx @@ -68,7 +68,7 @@ test('fetching requestable roles for servers uses UUID, not hostname', async () ); }); -test('fetching requestable roles kube_cluster resource without namespace request', async () => { +test('fetching requestable roles for a kube_cluster resource without specifying a namespace', async () => { const kube = makeKube(); const cluster = makeRootCluster(); const appContext = new MockAppContext(); diff --git a/web/packages/teleterm/src/ui/AccessRequestCheckout/useAccessRequestCheckout.ts b/web/packages/teleterm/src/ui/AccessRequestCheckout/useAccessRequestCheckout.ts index 363ef039d85cc..1d5a282bbdfd2 100644 --- a/web/packages/teleterm/src/ui/AccessRequestCheckout/useAccessRequestCheckout.ts +++ b/web/packages/teleterm/src/ui/AccessRequestCheckout/useAccessRequestCheckout.ts @@ -163,10 +163,6 @@ export default function useAccessRequestCheckout() { /** * * @param pendingRequest holds a list or map of resources to process - * @param excludeSubResourceParentResource when true, resources that have - * subresources, will be excluded from the returned list. eg: - * if a kube_cluster resource has a list of namespaces (subresources), - * then if this flag is true, kube_cluster will be excluded from the result. */ function getPendingAccessRequestsPerResource( pendingRequest: PendingAccessRequest @@ -293,19 +289,11 @@ export default function useAccessRequestCheckout() { resourceIds: pendingAccessRequestsWithoutParentResource .filter(d => d.kind !== 'role') .map(d => { - if (d.kind === 'namespace') { - return { - name: d.id, - kind: d.kind, - clusterName: d.clusterName, - subResourceName: d.subResourceName, - }; - } return { name: d.id, clusterName: d.clusterName, kind: d.kind, - subResourceName: '', + subResourceName: d.subResourceName || '', }; }), roles: pendingAccessRequestsWithoutParentResource @@ -345,6 +333,9 @@ export default function useAccessRequestCheckout() { async function performDryRun() { let teletermAccessRequest: TeletermAccessRequest; + // reset any create attempts between dry runs. + setCreateRequestAttempt({ status: '' }); + try { const { accessRequest } = await prepareAndCreateRequest({ dryRun: true, @@ -352,12 +343,9 @@ export default function useAccessRequestCheckout() { }); teletermAccessRequest = accessRequest; } catch { - setCreateRequestAttempt({ status: '' }); return; } - setCreateRequestAttempt({ status: '' }); - const accessRequest = makeUiAccessRequest(teletermAccessRequest); onDryRunChange(accessRequest); } @@ -440,7 +428,7 @@ export default function useAccessRequestCheckout() { isCollapsed, assumedRequests: getAssumedRequests(), toggleResource, - data: pendingAccessRequests, + pendingAccessRequests, shouldShowClusterNameColumn, createRequest, reset, diff --git a/web/packages/teleterm/src/ui/DocumentAccessRequests/NewRequest/useNewRequest.ts b/web/packages/teleterm/src/ui/DocumentAccessRequests/NewRequest/useNewRequest.ts index f9dde65ac760d..bcc9d7b04f3e0 100644 --- a/web/packages/teleterm/src/ui/DocumentAccessRequests/NewRequest/useNewRequest.ts +++ b/web/packages/teleterm/src/ui/DocumentAccessRequests/NewRequest/useNewRequest.ts @@ -211,9 +211,14 @@ export default function useNewRequest(rootCluster: Cluster) { return; } + /** + * This should never happen but just a safeguard. + * This function is used in the "unified resources" view, + * where a user can click on a "request access" button. + * Selecting kube_cluster's namespace is not available in this view + * (instead it is rendered in the "request checkout" view). + */ if (kind === 'namespace') { - // It is not possible to request a kube namespace through this function. - // The type should be corrected. return; }