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

Improves Rate Limit message accuracy, and fixes a bug with the deleted subscriptions status. #107

Merged
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
10 changes: 10 additions & 0 deletions changelog/v0.0.36/rate-limit-display-updated-when-mixed.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
changelog:
- type: FIX
issueLink: https://github.com/solo-io/solo-projects/issues/7043
description: >-
Updates the rate limit UI element to show mixed values when there is a App which
has multiple subscriptions with different rate limits.
- type: FIX
issueLink: https://github.com/solo-io/solo-projects/issues/7066
description: >-
Fixes a bug where subscriptions incorrectly showed up as deleted.
21 changes: 12 additions & 9 deletions projects/ui/src/Apis/api-types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@
// Gloo Mesh Gateway Types
//

import { getEnumValues } from "../Utility/utility";

type RateLimitPolicy = {
unit: "UNKNOWN" | "SECOND" | "MINUTE" | "HOUR" | "DAY";
requestsPerUnit: number;
Expand Down Expand Up @@ -171,25 +173,26 @@ export type OauthCredential = {
idpClientName: string;
};

// This list of units is used both for the type and for the dropdown in the UI.
const rateLimitUnits = [
export enum RateLimitUnit {
"UNKNOWN",
"SECOND",
"MINUTE",
"HOUR",
"DAY",
"MONTH",
"YEAR",
] as const; // The 'as const' tells TypeScript to treat these as literal types
export const rateLimitUnitOptions = rateLimitUnits.map((unit) => ({
value: unit,
label: unit,
}));
export type RateLimitUnit = (typeof rateLimitUnits)[number];
}
// This list of units is used both for the type and for the dropdown in the UI.
export const rateLimitUnitOptions = getEnumValues(RateLimitUnit).map(
(unit) => ({
value: RateLimitUnit[unit],
label: RateLimitUnit[unit],
})
);

export type RateLimit = {
requestsPerUnit: string;
unit: RateLimitUnit;
unit: string;
};

export type SubscriptionMetadata = {
Expand Down
8 changes: 5 additions & 3 deletions projects/ui/src/Apis/gg_hooks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ export function useListFlatAppsForTeamsOmitErrors(teams: Team[]) {
return { ...swrRes, data };
}
export function useGetAppDetails(id?: string) {
return useSwrWithAuth<App>(`/apps/${id}`);
return useSwrWithAuth<App>(`/apps/${id}`, id ?? null);
}
export function useListApiKeysForApp(appId: string) {
return useSwrWithAuth<ApiKey[]>(`/apps/${appId}/api-keys`);
Expand Down Expand Up @@ -101,9 +101,11 @@ export function useListSubscriptionsForStatus(status: SubscriptionStatus) {
}, [swrResponse]);
return swrResponse;
}
export function useListSubscriptionsForApp(appId: string) {
export function useListSubscriptionsForApp(appId: string | null) {
const endpoint = `/apps/${appId}/subscriptions`;
const swrResponse = useSwrWithAuth<Subscription[] | SubscriptionsListError>(
`/apps/${appId}/subscriptions`
endpoint,
appId === null ? null : endpoint
);
useEffect(() => {
if (isSubscriptionsListError(swrResponse.data)) {
Expand Down
2 changes: 1 addition & 1 deletion projects/ui/src/Apis/utility.ts
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ export async function fetchJSON(...args: Parameters<typeof fetch>) {
*/
export const useSwrWithAuth = <T>(
path: string,
swrKey?: string,
swrKey?: string | null,
config?: Parameters<typeof useSWR<T>>[2]
) => {
const { latestAccessToken } = useContext(AuthContext);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ import { App } from "../../../../Apis/api-types";
import { DetailsPageStyles } from "../../../../Styles/shared/DetailsPageStyles";
import { GridCardStyles } from "../../../../Styles/shared/GridCard.style";
import { MetadataDisplay } from "../../../../Utility/AdminUtility/MetadataDisplay";
import { EmptyData } from "../../../Common/EmptyData";

const AppMetadataSection = ({ app }: { app: App }) => {
//
Expand All @@ -14,17 +13,11 @@ const AppMetadataSection = ({ app }: { app: App }) => {
<DetailsPageStyles.Title>Metadata</DetailsPageStyles.Title>
<GridCardStyles.GridCard whiteBg wide>
<Box px={"20px"} py={"25px"}>
{!!app.metadata?.rateLimit ? (
<MetadataDisplay
item={app}
customMetadata={app.metadata?.customMetadata}
rateLimitInfo={app.metadata?.rateLimit}
/>
) : (
<Box pt="10px">
<EmptyData title="No App metadata was found." />
</Box>
)}
<MetadataDisplay
item={app}
customMetadata={app.metadata?.customMetadata}
rateLimitInfo={app.metadata?.rateLimit}
/>
</Box>
</GridCardStyles.GridCard>
</DetailsPageStyles.Section>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,6 @@ const SubscriptionInfoCardAdminFooter = ({
const [showRejectSubModal, setShowRejectSubModal] = useState(false);
const [showDeleteSubModal, setShowDeleteSubModal] = useState(false);

const canDeleteSubscription = subscriptionState !== SubscriptionState.DELETED;

//
// Render
//
Expand Down Expand Up @@ -50,7 +48,6 @@ const SubscriptionInfoCardAdminFooter = ({
<Button
color="danger"
size="xs"
disabled={!canDeleteSubscription}
onClick={() => setShowDeleteSubModal(true)}
>
Delete
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ export enum SubscriptionState {
PENDING,
APPROVED,
REJECTED,
DELETED,
}
export const subscriptionStateMap = {
[SubscriptionState.PENDING]: {
Expand All @@ -26,27 +25,15 @@ export const subscriptionStateMap = {
accentColor: colors.darkRed,
borderColor: colors.lightMidRed,
},
[SubscriptionState.DELETED]: {
subscriptionState: SubscriptionState.DELETED,
label: "DELETED",
accentColor: colors.aprilGrey,
borderColor: colors.aprilGrey,
},
};

const dateHasValue = (dateString: string | undefined) => {
return !!dateString && new Date(dateString).getFullYear() !== 0;
};

export const GetSubscriptionState = (subscription: Subscription) => {
if (!!subscription.approved) {
return SubscriptionState.APPROVED;
}
if (dateHasValue(subscription.deletedAt)) {
return SubscriptionState.DELETED;
}
if (!!subscription.rejected) {
return SubscriptionState.REJECTED;
}
// Deleted subscriptions aren't returned from the API.
return SubscriptionState.PENDING;
};
38 changes: 38 additions & 0 deletions projects/ui/src/Components/Common/WarningAlert.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
import { css } from "@emotion/react";
import styled from "@emotion/styled";
import { WarningExclamation } from "../../Assets/Icons/Icons";
import { borderRadiusConstants } from "../../Styles/constants";
import { Color, svgColorReplace } from "../../Styles/utils";

const AlertContainer = styled.div(
({ theme }) => css`
padding: 12px;
background-color: ${theme.lightYellow};
border-radius: ${borderRadiusConstants.small};

display: flex;
align-items: center;
justify-content: flex-start;

${svgColorReplace(theme.darkYellowDark20 as Color)};
> svg {
margin-right: 10px;
min-width: 24px;
}

* {
line-height: 1.3rem;
font-size: 0.95rem;
color: ${theme.darkYellowDark20};
}
`
);

export function WarningAlert(props: { children?: React.ReactNode }) {
return (
<AlertContainer>
<WarningExclamation width={"24px"} height={"24px"} />
<div>{props.children}</div>
</AlertContainer>
);
}
3 changes: 3 additions & 0 deletions projects/ui/src/Styles/colors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,10 @@ const colorMap = {
pumpkinOrangeLight10: Color(baseColors.pumpkinOrange).lighten(0.1).hex(),
pumpkinOrangeLight20: Color(baseColors.pumpkinOrange).lighten(0.2).hex(),

lightYellowLight1: Color(baseColors.lightYellow).lighten(0.02).hex(),
midYellowDark20: Color(baseColors.midYellow).darken(0.2).hex(),
darkYellowDark10: Color(baseColors.darkYellow).darken(0.1).hex(),
darkYellowDark20: Color(baseColors.darkYellow).darken(0.2).hex(),
} as const;

const semanticColorMap = {
Expand Down
2 changes: 1 addition & 1 deletion projects/ui/src/Styles/global-styles/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import "./style-reset.css";
// prettier-ignore
import "./fontFace.css";
// prettier-ignore
import "./graphiql.min.css";
// import "./graphiql.min.css";
// prettier-ignore
import "./highlight.js.min.css";

Expand Down
63 changes: 39 additions & 24 deletions projects/ui/src/Utility/AdminUtility/MetadataDisplay.tsx
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
import { Box } from "@mantine/core";
import { useEffect, useState } from "react";
import { App, RateLimit, Subscription } from "../../Apis/api-types";
import { useIsAdmin } from "../../Context/AuthContext";
import { useInArea } from "../utility";
import { CustomMetadataEditor } from "./CustomMetadataEditor";
import { RateLimitEditor } from "./RateLimitEditor";
import { RateLimitSection } from "./RateLimitSection";

export interface SharedMetadataProps {
item: App | Subscription;
Expand All @@ -21,37 +23,50 @@ export const MetadataDisplay = ({
}: SharedMetadataProps & {
onIsWideChange?: (newIsWide: boolean) => void;
}) => {
const isAdmin = useIsAdmin();
const [isEditingCustomMetadata, setIsEditingCustomMetadata] = useState(false);
const [isEditingRateLimit, setIsEditingRateLimit] = useState(false);

useEffect(() => {
onIsWideChange?.(isEditingCustomMetadata || isEditingCustomMetadata);
}, [isEditingCustomMetadata, isEditingRateLimit]);

const inAppDetailsPage = useInArea([`apps/${props.item.id}`]);
const isSubscription = "apiProductId" in props.item;

// This component is reused for apps and subscription rate limit & metadata.
// Here we show the rate limit when:
// - This is an admin
// - We are on the app details page
// - This is a subscription
const showingRateLimit = !!isAdmin || inAppDetailsPage || isSubscription;

return (
<Box sx={{ textAlign: "left" }}>
<Box mb={!!customMetadata || !!isEditingCustomMetadata ? "10px" : "0px"}>
<CustomMetadataEditor
isEditingCustomMetadata={isEditingCustomMetadata}
onIsEditingCustomMetadataChange={(value) =>
setIsEditingCustomMetadata(value)
}
customMetadata={customMetadata}
rateLimitInfo={rateLimitInfo}
{...props}
/>
</Box>
<Box mb="10px">
<RateLimitEditor
isEditingRateLimit={isEditingRateLimit}
onIsEditingRateLimitChange={(newIsEditingRateLimit) =>
setIsEditingRateLimit(newIsEditingRateLimit)
}
customMetadata={customMetadata}
rateLimitInfo={rateLimitInfo}
{...props}
/>
</Box>
<Box sx={{ textAlign: "left", width: "100%" }}>
<CustomMetadataEditor
isEditingCustomMetadata={isEditingCustomMetadata}
onIsEditingCustomMetadataChange={(value) =>
setIsEditingCustomMetadata(value)
}
customMetadata={customMetadata}
rateLimitInfo={rateLimitInfo}
{...props}
/>
{showingRateLimit && (
<Box mb="10px">
<RateLimitSection
inAppDetailsPage={inAppDetailsPage}
isSubscription={isSubscription}
isEditingRateLimit={isEditingRateLimit}
onIsEditingRateLimitChange={(newIsEditingRateLimit) =>
setIsEditingRateLimit(newIsEditingRateLimit)
}
customMetadata={customMetadata}
rateLimitInfo={rateLimitInfo}
{...props}
/>
</Box>
)}
</Box>
);
};
Loading
Loading