From fc0b4593f7e06e375adeaf03b89ec047eb0250d4 Mon Sep 17 00:00:00 2001 From: Riccardo Forina Date: Fri, 16 Feb 2024 20:25:47 +0100 Subject: [PATCH] Fix messages not refreshing (#489) * Fix messages not refreshing * Explicitly filter null and undefined values to avoid filtering valid values like 0 (zero) * Reduce the amount of messages per page users can select --- .../messages/ConnectedMessagesTable.tsx | 102 ++++++------------ .../messages/ConnectedRefreshSelector.tsx | 49 +++++++++ .../messages/_components/LimitSelector.tsx | 2 +- .../messages/_components/MessagesTable.tsx | 14 +-- .../topics/[topicId]/messages/page.tsx | 54 +++------- ui/components/RefreshSelector.tsx | 3 +- ui/utils/filterUndefinedFromObj.ts | 4 +- 7 files changed, 108 insertions(+), 120 deletions(-) create mode 100644 ui/app/[locale]/kafka/[kafkaId]/topics/[topicId]/messages/ConnectedRefreshSelector.tsx diff --git a/ui/app/[locale]/kafka/[kafkaId]/topics/[topicId]/messages/ConnectedMessagesTable.tsx b/ui/app/[locale]/kafka/[kafkaId]/topics/[topicId]/messages/ConnectedMessagesTable.tsx index a13a2056a..97f282891 100644 --- a/ui/app/[locale]/kafka/[kafkaId]/topics/[topicId]/messages/ConnectedMessagesTable.tsx +++ b/ui/app/[locale]/kafka/[kafkaId]/topics/[topicId]/messages/ConnectedMessagesTable.tsx @@ -1,18 +1,19 @@ "use client"; import { Message } from "@/api/messages/schema"; -import { RefreshInterval, RefreshSelector } from "@/components/RefreshSelector"; +import { ConnectedRefreshSelector } from "@/app/[locale]/kafka/[kafkaId]/topics/[topicId]/messages/ConnectedRefreshSelector"; +import { RefreshInterval } from "@/components/RefreshSelector"; import { useFilterParams } from "@/utils/useFilterParams"; -import { useEffect, useLayoutEffect, useState, useTransition } from "react"; -import { createPortal } from "react-dom"; +import { useCallback, useEffect, useState, useTransition } from "react"; import { MessagesTable } from "./_components/MessagesTable"; export function ConnectedMessagesTable({ - messages, - lastRefresh, + messages: initialData, + lastRefresh: initialRefresh, selectedMessage, partitions, params, + refresh, }: { messages: Message[]; lastRefresh: Date | undefined; @@ -26,11 +27,15 @@ export function ConnectedMessagesTable({ "filter[epoch]": number | undefined; limit: number; }; + refresh: () => Promise<{ messages: Message[]; ts: Date }>; }) { + const [{ messages, lastRefresh }, setMessages] = useState({ + messages: initialData, + lastRefresh: initialRefresh, + }); const [automaticRefresh, setAutomaticRefresh] = useState(false); const [refreshInterval, setRefreshInterval] = useState(1); const [isPending, startTransition] = useTransition(); - const [isAutorefreshing, startAutorefreshTransition] = useTransition(); const updateUrl = useFilterParams(params); function setPartition(partition: number | undefined) { @@ -113,25 +118,29 @@ export function ConnectedMessagesTable({ }); } - function refresh() { - startTransition(() => { - updateUrl({ ...params, _ts: Date.now() }); - }); - } + const doRefresh = useCallback( + async function doRefresh() { + const { messages, ts } = await refresh(); + startTransition(() => { + setMessages({ messages, lastRefresh: ts }); + }); + }, + [refresh], + ); useEffect(() => { - let interval: ReturnType; - if (automaticRefresh) { - interval = setInterval(async () => { - if (!isAutorefreshing) { - startAutorefreshTransition(() => - updateUrl({ ...params, _ts: Date.now() }), - ); - } - }, refreshInterval * 1000); + let t: ReturnType; + + async function tick() { + if (automaticRefresh) { + await doRefresh(); + } + t = setTimeout(tick, refreshInterval * 1000); } - return () => clearInterval(interval); - }, [params, updateUrl, automaticRefresh, refreshInterval, isAutorefreshing]); + + void tick(); + return () => clearTimeout(t); + }, [params, updateUrl, automaticRefresh, refreshInterval, doRefresh]); return ( <> @@ -155,61 +164,16 @@ export function ConnectedMessagesTable({ onSelectMessage={setSelected} onDeselectMessage={deselectMessage} /> + ); } - -function ConnectedRefreshSelector({ - isRefreshing, - isLive, - refreshInterval, - lastRefresh, - onRefresh, - onRefreshInterval, - onToggleLive, -}: { - isRefreshing: boolean; - isLive: boolean; - refreshInterval: RefreshInterval; - lastRefresh: Date | undefined; - onRefresh: () => void; - onRefreshInterval: (interval: RefreshInterval) => void; - onToggleLive: (enable: boolean) => void; -}) { - const [container, setContainer] = useState(); - - function seekContainer() { - const el = document.getElementById("topic-header-portal"); - if (el) { - setContainer(el); - } else { - setTimeout(seekContainer, 100); - } - } - - useLayoutEffect(seekContainer, []); // we want to run this just once - - return container - ? createPortal( - , - container, - ) - : null; -} diff --git a/ui/app/[locale]/kafka/[kafkaId]/topics/[topicId]/messages/ConnectedRefreshSelector.tsx b/ui/app/[locale]/kafka/[kafkaId]/topics/[topicId]/messages/ConnectedRefreshSelector.tsx new file mode 100644 index 000000000..7fcdb820c --- /dev/null +++ b/ui/app/[locale]/kafka/[kafkaId]/topics/[topicId]/messages/ConnectedRefreshSelector.tsx @@ -0,0 +1,49 @@ +import { RefreshInterval, RefreshSelector } from "@/components/RefreshSelector"; +import { useLayoutEffect, useState } from "react"; +import { createPortal } from "react-dom"; + +export function ConnectedRefreshSelector({ + isRefreshing, + isLive, + refreshInterval, + lastRefresh, + onRefresh, + onRefreshInterval, + onToggleLive, +}: { + isRefreshing: boolean; + isLive: boolean; + refreshInterval: RefreshInterval; + lastRefresh: Date | undefined; + onRefresh: () => void; + onRefreshInterval: (interval: RefreshInterval) => void; + onToggleLive: (enable: boolean) => void; +}) { + const [container, setContainer] = useState(); + + function seekContainer() { + const el = document.getElementById("topic-header-portal"); + if (el) { + setContainer(el); + } else { + setTimeout(seekContainer, 100); + } + } + + useLayoutEffect(seekContainer, []); // we want to run this just once + + return container + ? createPortal( + , + container, + ) + : null; +} diff --git a/ui/app/[locale]/kafka/[kafkaId]/topics/[topicId]/messages/_components/LimitSelector.tsx b/ui/app/[locale]/kafka/[kafkaId]/topics/[topicId]/messages/_components/LimitSelector.tsx index 5af271285..cb38e5651 100644 --- a/ui/app/[locale]/kafka/[kafkaId]/topics/[topicId]/messages/_components/LimitSelector.tsx +++ b/ui/app/[locale]/kafka/[kafkaId]/topics/[topicId]/messages/_components/LimitSelector.tsx @@ -36,7 +36,7 @@ export function LimitSelector({ )} > - {[20, 50, 100, 200, 500, 1000].map((value, idx) => ( + {[5, 10, 25, 50, 75, 100].map((value, idx) => ( onChange(value)}> {value} diff --git a/ui/app/[locale]/kafka/[kafkaId]/topics/[topicId]/messages/_components/MessagesTable.tsx b/ui/app/[locale]/kafka/[kafkaId]/topics/[topicId]/messages/_components/MessagesTable.tsx index 0ae01e06f..97b75afec 100644 --- a/ui/app/[locale]/kafka/[kafkaId]/topics/[topicId]/messages/_components/MessagesTable.tsx +++ b/ui/app/[locale]/kafka/[kafkaId]/topics/[topicId]/messages/_components/MessagesTable.tsx @@ -155,7 +155,6 @@ export function MessagesTable({ > @@ -418,7 +415,7 @@ export function MessagesTableToolbar({ > @@ -448,7 +445,7 @@ export function MessagesTableToolbar({ @@ -477,7 +474,6 @@ export function MessagesTableSkeleton({ aria-label={t("title")} > - } - > - - - ); -} - -async function ConnectedMessagesPage({ +export default async function ConnectedMessagesPage({ params: { kafkaId, topicId }, searchParams, }: { @@ -63,12 +33,17 @@ async function ConnectedMessagesPage({ epoch, } = parseSearchParams(searchParams); - const { messages, ts } = await getTopicMessages(kafkaId, topicId, { - pageSize: limit, - partition, - filter, - maxValueLength: 150, - }); + async function refresh(): Promise<{ messages: Message[]; ts: Date }> { + "use server"; + const { messages, ts } = await getTopicMessages(kafkaId, topicId, { + pageSize: limit, + partition, + filter, + maxValueLength: 150, + }); + return { messages, ts }; + } + const selectedMessage = selectedOffset !== undefined && selectedPartition !== undefined ? await getTopicMessage(kafkaId, topicId, { @@ -79,6 +54,8 @@ async function ConnectedMessagesPage({ const isFiltered = partition || epoch || offset || timestamp; + const { messages, ts } = await refresh(); + switch (true) { case !isFiltered && (messages === null || messages.length === 0): return ; @@ -97,6 +74,7 @@ async function ConnectedMessagesPage({ "filter[epoch]": epoch, "filter[offset]": offset, }} + refresh={refresh} /> ); } diff --git a/ui/components/RefreshSelector.tsx b/ui/components/RefreshSelector.tsx index 5b5aac0c5..f79abed19 100644 --- a/ui/components/RefreshSelector.tsx +++ b/ui/components/RefreshSelector.tsx @@ -57,7 +57,6 @@ export function RefreshSelector({ ref={toggleRef} onClick={toggleOpen} isExpanded={isOpen} - isDisabled={isRefreshing} variant={"default"} splitButtonOptions={{ variant: "action", @@ -98,7 +97,7 @@ export function RefreshSelector({ key={"refresh"} onClick={onRefresh} isRefreshing={isRefreshing} - isDisabled={isRefreshing} + isDisabled={isRefreshing || isLive} /> diff --git a/ui/utils/filterUndefinedFromObj.ts b/ui/utils/filterUndefinedFromObj.ts index 3ae52fce2..141a32b62 100644 --- a/ui/utils/filterUndefinedFromObj.ts +++ b/ui/utils/filterUndefinedFromObj.ts @@ -1,5 +1,7 @@ export function filterUndefinedFromObj(obj: Record) { return Object.fromEntries( - Object.entries(obj).filter(([_, value]) => !!value), + Object.entries(obj).filter( + ([_, value]) => value !== undefined && value !== null, + ), ); }