Skip to content

Commit

Permalink
Web: rename generic variable name to a helpful name (#47722)
Browse files Browse the repository at this point in the history
  • Loading branch information
kimlisa committed Nov 4, 2024
1 parent 178c922 commit fb3a889
Show file tree
Hide file tree
Showing 5 changed files with 51 additions and 31 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ export const Empty = () => {
<MemoryRouter>
<RequestCheckoutWithSlider
{...baseProps}
data={[]}
pendingAccessRequests={[]}
selectedReviewers={selectedReviewers}
setSelectedReviewers={setSelectedReviewers}
maxDuration={maxDuration}
Expand Down Expand Up @@ -177,7 +177,7 @@ const baseProps: RequestCheckoutWithSliderProps = {
],
setSelectedReviewers: () => null,
createRequest: () => null,
data: [
pendingAccessRequests: [
{
kind: 'app',
name: 'app-name',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,7 @@ const props: RequestCheckoutWithSliderProps = {
selectedReviewers: [],
setSelectedReviewers: () => null,
createRequest: () => null,
data: [],
pendingAccessRequests: [],
clearAttempt: () => null,
onClose: () => null,
toggleResource: () => null,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import {
ButtonPrimary,
ButtonSecondary,
Flex,
H2,
Image,
Indicator,
LabelInput,
Expand Down Expand Up @@ -138,7 +139,7 @@ export function RequestCheckout<T extends PendingListItem>({
setPendingRequestTtl,
pendingRequestTtlOptions,
dryRunResponse,
data,
pendingAccessRequests,
showClusterNameColumn,
createAttempt,
fetchResourceRequestRolesAttempt,
Expand Down Expand Up @@ -178,12 +179,14 @@ export function RequestCheckout<T extends PendingListItem>({
selectedResourceRequestRoles.length < 1;

const submitBtnDisabled =
data.length === 0 ||
pendingAccessRequests.length === 0 ||
createAttempt.status === 'processing' ||
isInvalidRoleSelection ||
fetchResourceRequestRolesAttempt.status === 'failed' ||
fetchResourceRequestRolesAttempt.status === 'processing';

const numPendingAccessRequests = pendingAccessRequests.length;

const DefaultHeader = () => {
return (
<Flex mb={3} alignItems="center">
Expand All @@ -195,9 +198,10 @@ export function RequestCheckout<T extends PendingListItem>({
style={{ cursor: 'pointer' }}
/>
<Box>
<Text typography="h4" color="text.main" bold>
{data.length} {pluralize(data.length, 'Resource')} Selected
</Text>
<H2>
{numPendingAccessRequests}{' '}
{pluralize(numPendingAccessRequests, 'Resource')} Selected
</H2>
</Box>
</Flex>
);
Expand Down Expand Up @@ -244,7 +248,7 @@ export function RequestCheckout<T extends PendingListItem>({
<Alert kind="danger" children={createAttempt.statusText} />
)}
<StyledTable
data={data}
data={pendingAccessRequests}
columns={[
{
key: 'clusterName',
Expand Down Expand Up @@ -754,7 +758,7 @@ export type RequestCheckoutProps<T extends PendingListItem = PendingListItem> =
isResourceRequest: boolean;
requireReason: boolean;
selectedReviewers: ReviewerOption[];
data: T[];
pendingAccessRequests: T[];
showClusterNameColumn?: boolean;
createRequest: (req: CreateRequest) => void;
fetchStatus: 'loading' | 'loaded';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ export function AccessRequestCheckout() {
fetchResourceRolesAttempt,
setSelectedResourceRequestRoles,
clearCreateAttempt,
data,
pendingAccessRequests,
shouldShowClusterNameColumn,
selectedReviewers,
setSelectedReviewers,
Expand All @@ -104,18 +104,23 @@ export function AccessRequestCheckout() {
onStartTimeChange,
} = useAccessRequestCheckout();

const isRoleRequest = data[0]?.kind === 'role';
const isRoleRequest = pendingAccessRequests[0]?.kind === 'role';

function closeCheckout() {
setShowCheckout(false);
}

// We should rather detect how much space we have,
// but for simplicity we only count items.
const moreToShow = Math.max(data.length - MAX_RESOURCES_IN_BAR_TO_SHOW, 0);
const moreToShow = Math.max(
pendingAccessRequests.length - MAX_RESOURCES_IN_BAR_TO_SHOW,
0
);
const numPendingAccessRequests = pendingAccessRequests.length;

return (
<>
{data.length > 0 && !isCollapsed() && (
{pendingAccessRequests.length > 0 && !isCollapsed() && (
<Box
px={3}
py={2}
Expand All @@ -133,12 +138,15 @@ export function AccessRequestCheckout() {
>
<Flex flexDirection="column" minWidth={0}>
<Text mb={1}>
{data.length}{' '}
{pluralize(data.length, isRoleRequest ? 'role' : 'resource')}{' '}
{numPendingAccessRequests}{' '}
{pluralize(
numPendingAccessRequests,
isRoleRequest ? 'role' : 'resource'
)}{' '}
added to access request:
</Text>
<Flex gap={1} flexWrap="wrap">
{data
{pendingAccessRequests
.slice(0, MAX_RESOURCES_IN_BAR_TO_SHOW)
.map(c => {
let resource = {
Expand Down Expand Up @@ -234,7 +242,7 @@ export function AccessRequestCheckout() {
})
}
reset={reset}
data={data}
pendingAccessRequests={pendingAccessRequests}
showClusterNameColumn={shouldShowClusterNameColumn}
createAttempt={createRequestAttempt}
resourceRequestRoles={resourceRequestRoles}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ export default function useAccessRequestCheckout() {
workspaceAccessRequest?.getPendingAccessRequest();

useEffect(() => {
// Do a new dry run per changes to pending data
// Do a new dry run per changes to pending access requests
// to get the latest time options and latest calculated
// suggested reviewers.
// Options and reviewers can change depending on the selected
Expand All @@ -106,12 +106,13 @@ export default function useAccessRequestCheckout() {
return;
}

const data = getPendingAccessRequestsPerResource(pendingAccessRequest);
const pendingAccessRequests =
getPendingAccessRequestsPerResource(pendingAccessRequest);
runFetchResourceRoles(() =>
retryWithRelogin(ctx, clusterUri, async () => {
const { response } = await ctx.tshd.getRequestableRoles({
clusterUri: rootClusterUri,
resourceIds: data
resourceIds: pendingAccessRequests
.filter(d => d.kind !== 'role')
.map(d => ({
// We have to use id, not name.
Expand Down Expand Up @@ -148,17 +149,17 @@ export default function useAccessRequestCheckout() {
function getPendingAccessRequestsPerResource(
pendingRequest: PendingAccessRequest
): PendingListItemWithOriginalItem[] {
const data: PendingListItemWithOriginalItem[] = [];
const pendingAccessRequests: PendingListItemWithOriginalItem[] = [];
if (!workspaceAccessRequest) {
return data;
return pendingAccessRequests;
}

switch (pendingRequest.kind) {
case 'role': {
const clusterName =
ctx.clustersService.findCluster(rootClusterUri)?.name;
pendingRequest.roles.forEach(role => {
data.push({
pendingAccessRequests.push({
kind: 'role',
id: role,
name: role,
Expand All @@ -171,7 +172,7 @@ export default function useAccessRequestCheckout() {
pendingRequest.resources.forEach(resourceRequest => {
const { kind, id, name } =
extractResourceRequestProperties(resourceRequest);
data.push({
pendingAccessRequests.push({
kind,
id,
name,
Expand All @@ -183,7 +184,7 @@ export default function useAccessRequestCheckout() {
});
}
}
return data;
return pendingAccessRequests;
}

function isCollapsed() {
Expand Down Expand Up @@ -221,21 +222,24 @@ export default function useAccessRequestCheckout() {
* Shared logic used both during dry runs and regular access request creation.
*/
function prepareAndCreateRequest(req: CreateRequest) {
const data = getPendingAccessRequestsPerResource(pendingAccessRequest);
const pendingAccessRequests =
getPendingAccessRequestsPerResource(pendingAccessRequest);
const params: CreateAccessRequestRequest = {
rootClusterUri,
reason: req.reason,
suggestedReviewers: req.suggestedReviewers || [],
dryRun: req.dryRun,
resourceIds: data
resourceIds: pendingAccessRequests
.filter(d => d.kind !== 'role')
.map(d => ({
name: d.id,
clusterName: d.clusterName,
kind: d.kind,
subResourceName: '',
})),
roles: data.filter(d => d.kind === 'role').map(d => d.name),
roles: pendingAccessRequests
.filter(d => d.kind === 'role')
.map(d => d.name),
assumeStartTime: req.start && Timestamp.fromDate(req.start),
maxDuration: req.maxDuration && Timestamp.fromDate(req.maxDuration),
requestTtl: req.requestTTL && Timestamp.fromDate(req.requestTTL),
Expand All @@ -250,7 +254,10 @@ export default function useAccessRequestCheckout() {

return retryWithRelogin(ctx, clusterUri, () =>
ctx.clustersService.createAccessRequest(params).then(({ response }) => {
return { accessRequest: response.request, requestedCount: data.length };
return {
accessRequest: response.request,
requestedCount: pendingAccessRequests.length,
};
})
).catch(e => {
setCreateRequestAttempt({ status: 'failed', statusText: e.message });
Expand Down Expand Up @@ -337,7 +344,8 @@ export default function useAccessRequestCheckout() {
isCollapsed,
assumedRequests: getAssumedRequests(),
toggleResource,
data: getPendingAccessRequestsPerResource(pendingAccessRequest),
pendingAccessRequests:
getPendingAccessRequestsPerResource(pendingAccessRequest),
shouldShowClusterNameColumn,
createRequest,
reset,
Expand Down

0 comments on commit fb3a889

Please sign in to comment.