From 28b06fc6e1598391a62c9f086230493f11508b9a Mon Sep 17 00:00:00 2001 From: Nicholas Bucher Date: Tue, 22 Oct 2024 11:41:04 -0400 Subject: [PATCH 1/5] changelog Signed-off-by: Nicholas Bucher --- .../v0.0.36/rate-limit-display-updated-when-mixed.yaml | 6 ++++++ 1 file changed, 6 insertions(+) create mode 100644 changelog/v0.0.36/rate-limit-display-updated-when-mixed.yaml diff --git a/changelog/v0.0.36/rate-limit-display-updated-when-mixed.yaml b/changelog/v0.0.36/rate-limit-display-updated-when-mixed.yaml new file mode 100644 index 00000000..c96d5b14 --- /dev/null +++ b/changelog/v0.0.36/rate-limit-display-updated-when-mixed.yaml @@ -0,0 +1,6 @@ +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. From 3b9f7ebe490d1721a0b7178bc71d170c46cd6941 Mon Sep 17 00:00:00 2001 From: Nicholas Bucher Date: Tue, 22 Oct 2024 16:35:05 -0400 Subject: [PATCH 2/5] added logic in for showing alerts on the app rate limit sections. Signed-off-by: Nicholas Bucher --- projects/ui/src/Apis/api-types.ts | 21 +++--- projects/ui/src/Apis/gg_hooks.ts | 8 ++- projects/ui/src/Apis/utility.ts | 2 +- .../ui/src/Components/Common/WarningAlert.tsx | 38 +++++++++++ projects/ui/src/Styles/colors.ts | 3 + .../Utility/AdminUtility/MetadataDisplay.tsx | 51 +++++++++++---- .../Utility/AdminUtility/RateLimitEditor.tsx | 23 ++++--- .../Utility/AdminUtility/RateLimitSection.tsx | 64 +++++++++++++++++++ 8 files changed, 173 insertions(+), 37 deletions(-) create mode 100644 projects/ui/src/Components/Common/WarningAlert.tsx create mode 100644 projects/ui/src/Utility/AdminUtility/RateLimitSection.tsx diff --git a/projects/ui/src/Apis/api-types.ts b/projects/ui/src/Apis/api-types.ts index b162e869..ddbfb1f8 100644 --- a/projects/ui/src/Apis/api-types.ts +++ b/projects/ui/src/Apis/api-types.ts @@ -2,6 +2,8 @@ // Gloo Mesh Gateway Types // +import { getEnumValues } from "../Utility/utility"; + type RateLimitPolicy = { unit: "UNKNOWN" | "SECOND" | "MINUTE" | "HOUR" | "DAY"; requestsPerUnit: number; @@ -171,8 +173,7 @@ 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", @@ -180,16 +181,18 @@ const rateLimitUnits = [ "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 = { diff --git a/projects/ui/src/Apis/gg_hooks.ts b/projects/ui/src/Apis/gg_hooks.ts index 33732e14..979b6df0 100644 --- a/projects/ui/src/Apis/gg_hooks.ts +++ b/projects/ui/src/Apis/gg_hooks.ts @@ -56,7 +56,7 @@ export function useListFlatAppsForTeamsOmitErrors(teams: Team[]) { return { ...swrRes, data }; } export function useGetAppDetails(id?: string) { - return useSwrWithAuth(`/apps/${id}`); + return useSwrWithAuth(`/apps/${id}`, id ?? null); } export function useListApiKeysForApp(appId: string) { return useSwrWithAuth(`/apps/${appId}/api-keys`); @@ -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( - `/apps/${appId}/subscriptions` + endpoint, + appId === null ? null : endpoint ); useEffect(() => { if (isSubscriptionsListError(swrResponse.data)) { diff --git a/projects/ui/src/Apis/utility.ts b/projects/ui/src/Apis/utility.ts index ff180ea6..e37329bd 100644 --- a/projects/ui/src/Apis/utility.ts +++ b/projects/ui/src/Apis/utility.ts @@ -69,7 +69,7 @@ export async function fetchJSON(...args: Parameters) { */ export const useSwrWithAuth = ( path: string, - swrKey?: string, + swrKey?: string | null, config?: Parameters>[2] ) => { const { latestAccessToken } = useContext(AuthContext); diff --git a/projects/ui/src/Components/Common/WarningAlert.tsx b/projects/ui/src/Components/Common/WarningAlert.tsx new file mode 100644 index 00000000..d7b8adfd --- /dev/null +++ b/projects/ui/src/Components/Common/WarningAlert.tsx @@ -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 ( + + +
{props.children}
+ + ); +} diff --git a/projects/ui/src/Styles/colors.ts b/projects/ui/src/Styles/colors.ts index d8c35710..b24d72fa 100644 --- a/projects/ui/src/Styles/colors.ts +++ b/projects/ui/src/Styles/colors.ts @@ -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 = { diff --git a/projects/ui/src/Utility/AdminUtility/MetadataDisplay.tsx b/projects/ui/src/Utility/AdminUtility/MetadataDisplay.tsx index e9792125..514610ae 100644 --- a/projects/ui/src/Utility/AdminUtility/MetadataDisplay.tsx +++ b/projects/ui/src/Utility/AdminUtility/MetadataDisplay.tsx @@ -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; @@ -21,6 +23,7 @@ export const MetadataDisplay = ({ }: SharedMetadataProps & { onIsWideChange?: (newIsWide: boolean) => void; }) => { + const isAdmin = useIsAdmin(); const [isEditingCustomMetadata, setIsEditingCustomMetadata] = useState(false); const [isEditingRateLimit, setIsEditingRateLimit] = useState(false); @@ -28,9 +31,25 @@ export const MetadataDisplay = ({ 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 ( - - + + @@ -41,17 +60,21 @@ export const MetadataDisplay = ({ {...props} /> - - - setIsEditingRateLimit(newIsEditingRateLimit) - } - customMetadata={customMetadata} - rateLimitInfo={rateLimitInfo} - {...props} - /> - + {showingRateLimit && ( + + + setIsEditingRateLimit(newIsEditingRateLimit) + } + customMetadata={customMetadata} + rateLimitInfo={rateLimitInfo} + {...props} + /> + + )} ); }; diff --git a/projects/ui/src/Utility/AdminUtility/RateLimitEditor.tsx b/projects/ui/src/Utility/AdminUtility/RateLimitEditor.tsx index 22f4a5f1..751586f0 100644 --- a/projects/ui/src/Utility/AdminUtility/RateLimitEditor.tsx +++ b/projects/ui/src/Utility/AdminUtility/RateLimitEditor.tsx @@ -13,23 +13,27 @@ import { useIsAdmin } from "../../Context/AuthContext"; import { shallowEquals } from "../utility"; import { SharedMetadataProps } from "./MetadataDisplay"; +export type RateLimitEditorProps = SharedMetadataProps & { + inAppDetailsPage: boolean; + isSubscription: boolean; + isEditingRateLimit: boolean; + onIsEditingRateLimitChange: (newIsEditinRateLimit: boolean) => void; +}; + export const RateLimitEditor = ({ item, isEditingRateLimit, customMetadata, rateLimitInfo, onIsEditingRateLimitChange, -}: SharedMetadataProps & { - isEditingRateLimit: boolean; - onIsEditingRateLimitChange: (newIsEditinRateLimit: boolean) => void; -}) => { +}: RateLimitEditorProps) => { // // region State // const initialRateLimitInfo = rateLimitInfo ?? { // This is the default if no rate limit is specified. requestsPerUnit: "0", - unit: "UNKNOWN", + unit: RateLimitUnit[RateLimitUnit.UNKNOWN], }; let initialRPU = 0; try { @@ -129,7 +133,7 @@ export const RateLimitEditor = ({ {/* // region Edit/Save Buttons */} - {isAdmin ? ( + {isAdmin && ( + ) : ( + + )} + + {/* Admins will see this message when editing the App Rate Limits. */} + {!props.isSubscription && !!appSubscriptions && ( + + + Please note that Subscriptions are able to override this Apps Rate + Limit value. + + {Array.isArray(appSubscriptions) && ( + <> + This App has {appSubscriptions.length} Subscription + {appSubscriptions.length === 1 ? "" : "s"}. + + )} + + + )} + + {/* Non-admins will see this message on their App details pages. */} + {!props.isSubscription && !!props.inAppDetailsPage && !isAdmin && ( + + + Please note that Subscriptions are able to override this Apps Rate + Limit value. + + + )} + + ); +}; From e572fc40a21bea01b35e043ae3b0e33a1cce29b7 Mon Sep 17 00:00:00 2001 From: Nicholas Bucher Date: Wed, 23 Oct 2024 11:22:15 -0400 Subject: [PATCH 3/5] Added an empty rate limit message, and updated changelog. Signed-off-by: Nicholas Bucher --- ...rate-limit-display-updated-when-mixed.yaml | 4 + .../MetadataSection/AppMetadataSection.tsx | 23 ++-- .../SubscriptionInfoCardAdminFooter.tsx | 3 - .../SubscriptionsList/SubscriptionsUtility.ts | 15 +-- projects/ui/src/Styles/global-styles/index.ts | 2 +- .../Utility/AdminUtility/RateLimitEditor.tsx | 102 ++++++++++-------- 6 files changed, 73 insertions(+), 76 deletions(-) diff --git a/changelog/v0.0.36/rate-limit-display-updated-when-mixed.yaml b/changelog/v0.0.36/rate-limit-display-updated-when-mixed.yaml index c96d5b14..4eeebb1b 100644 --- a/changelog/v0.0.36/rate-limit-display-updated-when-mixed.yaml +++ b/changelog/v0.0.36/rate-limit-display-updated-when-mixed.yaml @@ -4,3 +4,7 @@ changelog: 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. diff --git a/projects/ui/src/Components/Apps/Details/MetadataSection/AppMetadataSection.tsx b/projects/ui/src/Components/Apps/Details/MetadataSection/AppMetadataSection.tsx index 3e41de86..08ee9c95 100644 --- a/projects/ui/src/Components/Apps/Details/MetadataSection/AppMetadataSection.tsx +++ b/projects/ui/src/Components/Apps/Details/MetadataSection/AppMetadataSection.tsx @@ -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 }) => { // @@ -14,17 +13,17 @@ const AppMetadataSection = ({ app }: { app: App }) => { Metadata - {!!app.metadata?.rateLimit ? ( - - ) : ( - - - - )} + {/* {!!app.metadata?.rateLimit ? ( */} + + {/* // ) : ( + // + // + // + // )} */} diff --git a/projects/ui/src/Components/Common/SubscriptionsList/SubscriptionInfoCard/SubscriptionInfoCardAdminFooter.tsx b/projects/ui/src/Components/Common/SubscriptionsList/SubscriptionInfoCard/SubscriptionInfoCardAdminFooter.tsx index 9876d793..595ad44a 100644 --- a/projects/ui/src/Components/Common/SubscriptionsList/SubscriptionInfoCard/SubscriptionInfoCardAdminFooter.tsx +++ b/projects/ui/src/Components/Common/SubscriptionsList/SubscriptionInfoCard/SubscriptionInfoCardAdminFooter.tsx @@ -19,8 +19,6 @@ const SubscriptionInfoCardAdminFooter = ({ const [showRejectSubModal, setShowRejectSubModal] = useState(false); const [showDeleteSubModal, setShowDeleteSubModal] = useState(false); - const canDeleteSubscription = subscriptionState !== SubscriptionState.DELETED; - // // Render // @@ -50,7 +48,6 @@ const SubscriptionInfoCardAdminFooter = ({