fix(sdk): Split `useSummarizeQuery` into specialized hooks (#49841) (#50029)

Co-authored-by: Oisin Coveney <oisin@metabase.com>

enterprise/frontend/src/embedding-sdk/components/private/InteractiveQuestion/components/Summarize.tsx

 import { useRef, useState } from "react";
 import { t } from "ttag";
 
-import { useInteractiveQuestionContext } from "embedding-sdk/components/private/InteractiveQuestion/context";
 import CS from "metabase/css/core/index.css";
 import {
   SummarizeAggregationItemList,
   SummarizeBreakoutColumnList,
-  useSummarizeQuery,
 } from "metabase/query_builder/components/view/sidebars/SummarizeSidebar/SummarizeContent";
 import { Button, Divider, Group, Stack } from "metabase/ui";
-import type * as Lib from "metabase-lib";
+import * as Lib from "metabase-lib";
 import type Question from "metabase-lib/v1/Question";
 
+import { useInteractiveQuestionContext } from "../context";
+
 type SummarizeProps = {
   onClose: () => void;
 };
@@ -38,8 +38,11 @@ const SummarizeInner = ({
 
   const [currentQuery, setCurrentQuery] = useState<Lib.Query>(question.query());
 
+  // yeah we need to change this
+  const stageIndex = Lib.stageCount(currentQuery);
+
   const onApplyFilter = () => {
-    if (query) {
+    if (currentQuery) {
       onQueryChange(currentQuery);
       onClose();
     }
@@ -52,39 +55,22 @@ const SummarizeInner = ({
     onClose();
   };
 
-  const {
-    query,
-    stageIndex,
-    aggregations,
-    handleAddBreakout,
-    handleQueryChange,
-    handleRemoveBreakout,
-    handleReplaceBreakouts,
-    handleUpdateBreakout,
-    hasAggregations,
-  } = useSummarizeQuery({
-    query: currentQuery,
-    onQueryChange: setCurrentQuery,
-  });
+  const hasAggregations = Lib.aggregations(currentQuery, stageIndex).length > 0;
 
   return (
     <Stack className={CS.overflowHidden} h="100%" w="100%">
       <Stack className={CS.overflowYScroll}>
         <SummarizeAggregationItemList
-          query={query}
+          query={currentQuery}
+          onQueryChange={setCurrentQuery}
           stageIndex={stageIndex}
-          aggregations={aggregations}
-          onQueryChange={handleQueryChange}
         />
         <Divider my="lg" />
         {hasAggregations && (
           <SummarizeBreakoutColumnList
-            query={query}
+            query={currentQuery}
+            onQueryChange={setCurrentQuery}
             stageIndex={stageIndex}
-            onAddBreakout={handleAddBreakout}
-            onUpdateBreakout={handleUpdateBreakout}
-            onRemoveBreakout={handleRemoveBreakout}
-            onReplaceBreakouts={handleReplaceBreakouts}
           />
         )}
       </Stack>

frontend/src/metabase/query_builder/components/view/View/StructuredQueryRightSidebar/StructuredQueryRightSidebar.jsx

@@ -49,6 +49,7 @@ export const StructuredQueryRightSidebar = ({
             });
           }}
           onClose={onCloseSummary}
+          stageIndex={-1}
         />
       ),
     )

frontend/src/metabase/query_builder/components/view/sidebars/SummarizeSidebar/AggregationItem/AggregationItem.tsx

-import { useCallback, useState } from "react";
+import { useDisclosure } from "@mantine/hooks";
 
 import { AggregationPicker } from "metabase/common/components/AggregationPicker";
 import { Popover } from "metabase/ui";
-import * as Lib from "metabase-lib";
+import type * as Lib from "metabase-lib";
 
 import { AggregationName, RemoveIcon, Root } from "./AggregationItem.styled";
 
 interface AggregationItemProps {
   query: Lib.Query;
+  onQueryChange: (query: Lib.Query) => void;
   stageIndex: number;
   aggregation: Lib.AggregationClause;
   aggregationIndex: number;
-  onQueryChange: (query: Lib.Query) => void;
+  displayName: string;
+  onAggregationRemove: () => void;
+  operators: Lib.AggregationOperator[];
 }
 
 export function AggregationItem({
@@ -20,30 +23,22 @@ export function AggregationItem({
   aggregation,
   aggregationIndex,
   onQueryChange,
+  displayName,
+  onAggregationRemove,
+  operators,
 }: AggregationItemProps) {
-  const [isOpened, setIsOpened] = useState(false);
-  const { displayName } = Lib.displayInfo(query, stageIndex, aggregation);
-
-  const operators = Lib.selectedAggregationOperators(
-    Lib.availableAggregationOperators(query, stageIndex),
-    aggregation,
-  );
-
-  const handleRemove = useCallback(() => {
-    const nextQuery = Lib.removeClause(query, stageIndex, aggregation);
-    onQueryChange(nextQuery);
-  }, [query, stageIndex, aggregation, onQueryChange]);
+  const [isOpened, { toggle }] = useDisclosure(false);
 
   return (
-    <Popover opened={isOpened} onChange={setIsOpened}>
+    <Popover opened={isOpened} onChange={toggle}>
       <Popover.Target>
         <Root
           aria-label={displayName}
           data-testid="aggregation-item"
-          onClick={() => setIsOpened(!isOpened)}
+          onClick={toggle}
         >
           <AggregationName>{displayName}</AggregationName>
-          <RemoveIcon name="close" onClick={handleRemove} />
+          <RemoveIcon name="close" onClick={onAggregationRemove} />
         </Root>
       </Popover.Target>
       <Popover.Dropdown>

frontend/src/metabase/query_builder/components/view/sidebars/SummarizeSidebar/BreakoutColumnList/BreakoutColumnList.tsx

@@ -4,32 +4,28 @@ import { t } from "ttag";
 import Input from "metabase/core/components/Input";
 import { useDebouncedValue } from "metabase/hooks/use-debounced-value";
 import { SEARCH_DEBOUNCE_DURATION } from "metabase/lib/constants";
+import type { UpdateQueryHookProps } from "metabase/query_builder/hooks/types";
+import { useBreakoutQueryHandlers } from "metabase/query_builder/hooks/use-breakout-query-handlers";
 import { DelayGroup } from "metabase/ui";
 import * as Lib from "metabase-lib";
 
 import { ColumnGroupName, SearchContainer } from "./BreakoutColumnList.styled";
 import { BreakoutColumnListItem } from "./BreakoutColumnListItem";
 
-export interface BreakoutColumnListProps {
-  query: Lib.Query;
-  stageIndex: number;
-  onAddBreakout: (column: Lib.ColumnMetadata) => void;
-  onUpdateBreakout: (
-    breakout: Lib.BreakoutClause,
-    column: Lib.ColumnMetadata,
-  ) => void;
-  onRemoveBreakout: (breakout: Lib.BreakoutClause) => void;
-  onReplaceBreakouts: (column: Lib.ColumnMetadata) => void;
-}
+export type BreakoutColumnListProps = UpdateQueryHookProps;
 
 export function BreakoutColumnList({
   query,
-  stageIndex,
-  onAddBreakout,
-  onUpdateBreakout,
-  onRemoveBreakout,
-  onReplaceBreakouts,
+  onQueryChange,
+  stageIndex = -1,
 }: BreakoutColumnListProps) {
+  const {
+    onAddBreakout,
+    onUpdateBreakout,
+    onRemoveBreakout,
+    onReplaceBreakouts,
+  } = useBreakoutQueryHandlers({ query, onQueryChange, stageIndex });
+
   const [searchQuery, setSearchQuery] = useState("");
   const debouncedSearchQuery = useDebouncedValue(
     searchQuery,

frontend/src/metabase/query_builder/components/view/sidebars/SummarizeSidebar/SummarizeContent/SummarizeAggregationItemList.tsx

+import { useMemo } from "react";
+
+import type { UpdateQueryHookProps } from "metabase/query_builder/hooks/types";
+import { getAggregationItems } from "metabase/query_builder/utils/get-aggregation-items";
 import { Group, type GroupProps } from "metabase/ui";
-import type * as Lib from "metabase-lib";
+import * as Lib from "metabase-lib";
 
 import { AddAggregationButton } from "../AddAggregationButton";
 import { AggregationItem } from "../AggregationItem";
 
-type SummarizeAggregationItemListProps = {
-  query: Lib.Query;
-  stageIndex: number;
-  aggregations: Lib.AggregationClause[];
-  onQueryChange: (query: Lib.Query) => void;
-} & GroupProps;
+type SummarizeAggregationItemListProps = UpdateQueryHookProps & GroupProps;
 
 export const SummarizeAggregationItemList = ({
   query,
-  stageIndex,
-  aggregations,
   onQueryChange,
+  stageIndex,
   ...containerProps
-}: SummarizeAggregationItemListProps) => (
-  <Group
-    data-testid="summarize-aggregation-item-list"
-    spacing="sm"
-    align="flex-start"
-    {...containerProps}
-  >
-    {aggregations.map((aggregation, aggregationIndex) => (
-      <AggregationItem
-        key={aggregationIndex}
+}: SummarizeAggregationItemListProps) => {
+  const aggregationItems = useMemo(
+    () => getAggregationItems({ query, stageIndex }),
+    [query, stageIndex],
+  );
+
+  const handleRemove = (aggregation: Lib.AggregationClause) => {
+    const nextQuery = Lib.removeClause(query, stageIndex, aggregation);
+    onQueryChange(nextQuery);
+  };
+
+  return (
+    <Group
+      data-testid="summarize-aggregation-item-list"
+      spacing="sm"
+      align="flex-start"
+      {...containerProps}
+    >
+      {aggregationItems.map(
+        ({ aggregation, displayName, aggregationIndex, operators }) => (
+          <AggregationItem
+            key={aggregationIndex}
+            query={query}
+            stageIndex={stageIndex}
+            aggregation={aggregation}
+            aggregationIndex={aggregationIndex}
+            onQueryChange={onQueryChange}
+            displayName={displayName}
+            onAggregationRemove={() => handleRemove(aggregation)}
+            operators={operators}
+          />
+        ),
+      )}
+      <AddAggregationButton
         query={query}
         stageIndex={stageIndex}
-        aggregation={aggregation}
-        aggregationIndex={aggregationIndex}
         onQueryChange={onQueryChange}
       />
-    ))}
-    <AddAggregationButton
-      query={query}
-      stageIndex={stageIndex}
-      onQueryChange={onQueryChange}
-    />
-  </Group>
-);
+    </Group>
+  );
+};

frontend/src/metabase/query_builder/components/view/sidebars/SummarizeSidebar/SummarizeContent/SummarizeBreakoutColumnList.tsx

 import { t } from "ttag";
 
+import type { UpdateQueryHookProps } from "metabase/query_builder/hooks/types";
 import { Space, Stack, type StackProps, Title } from "metabase/ui";
-import type * as Lib from "metabase-lib";
 
 import { BreakoutColumnList } from "../BreakoutColumnList";
 
-type SummarizeBreakoutColumnListProps = {
-  query: Lib.Query;
-  stageIndex: number;
-  onAddBreakout: (column: Lib.ColumnMetadata) => void;
-  onUpdateBreakout: (
-    clause: Lib.BreakoutClause,
-    column: Lib.ColumnMetadata,
-  ) => void;
-  onRemoveBreakout: (clause: Lib.BreakoutClause) => void;
-  onReplaceBreakouts: (column: Lib.ColumnMetadata) => void;
-} & StackProps;
+type SummarizeBreakoutColumnListProps = UpdateQueryHookProps & StackProps;
 
 export const SummarizeBreakoutColumnList = ({
   query,
+  onQueryChange,
   stageIndex,
-  onAddBreakout,
-  onUpdateBreakout,
-  onRemoveBreakout,
-  onReplaceBreakouts,
   ...containerProps
 }: SummarizeBreakoutColumnListProps) => (
   <Stack
@@ -36,11 +23,8 @@ export const SummarizeBreakoutColumnList = ({
     <Space my="sm" />
     <BreakoutColumnList
       query={query}
+      onQueryChange={onQueryChange}
       stageIndex={stageIndex}
-      onAddBreakout={onAddBreakout}
-      onUpdateBreakout={onUpdateBreakout}
-      onRemoveBreakout={onRemoveBreakout}
-      onReplaceBreakouts={onReplaceBreakouts}
     />
   </Stack>
 );

frontend/src/metabase/query_builder/components/view/sidebars/SummarizeSidebar/SummarizeContent/index.ts

-export * from "./use-summarize-query";
 export * from "./SummarizeAggregationItemList";
 export * from "./SummarizeBreakoutColumnList";

frontend/src/metabase/query_builder/components/view/sidebars/SummarizeSidebar/SummarizeContent/use-summarize-query.ts

-import { useCallback, useMemo, useState } from "react";
-
-import * as Lib from "metabase-lib";
-
-const STAGE_INDEX = -1;
-
-interface UseSummarizeQueryProps {
-  query: Lib.Query;
-  onQueryChange: (nextQuery: Lib.Query) => void;
-}
-
-export const useSummarizeQuery = ({
-  query: initialQuery,
-  onQueryChange,
-}: UseSummarizeQueryProps) => {
-  const [hasDefaultAggregation, setHasDefaultAggregation] = useState(() =>
-    shouldAddDefaultAggregation(initialQuery),
-  );
-
-  const query = useMemo(
-    () =>
-      hasDefaultAggregation
-        ? Lib.aggregateByCount(initialQuery, STAGE_INDEX)
-        : initialQuery,
-    [initialQuery, hasDefaultAggregation],
-  );
-
-  const aggregations = Lib.aggregations(query, STAGE_INDEX);
-  const hasAggregations = aggregations.length > 0;
-
-  const handleChange = useCallback(
-    (nextQuery: Lib.Query) => {
-      setHasDefaultAggregation(false);
-      onQueryChange(nextQuery);
-    },
-    [onQueryChange],
-  );
-
-  const handleQueryChange = useCallback(
-    (nextQuery: Lib.Query) => {
-      const newAggregations = Lib.aggregations(nextQuery, STAGE_INDEX);
-      if (hasDefaultAggregation && newAggregations.length === 0) {
-        setHasDefaultAggregation(false);
-      } else {
-        handleChange(nextQuery);
-      }
-    },
-    [handleChange, hasDefaultAggregation],
-  );
-
-  const handleAddBreakout = useCallback(
-    (column: Lib.ColumnMetadata) => {
-      const nextQuery = Lib.breakout(query, STAGE_INDEX, column);
-      handleChange(nextQuery);
-    },
-    [query, handleChange],
-  );
-
-  const handleUpdateBreakout = useCallback(
-    (clause: Lib.BreakoutClause, column: Lib.ColumnMetadata) => {
-      const nextQuery = Lib.replaceClause(query, STAGE_INDEX, clause, column);
-      handleChange(nextQuery);
-    },
-    [query, handleChange],
-  );
-
-  const handleRemoveBreakout = useCallback(
-    (clause: Lib.BreakoutClause) => {
-      const nextQuery = Lib.removeClause(query, STAGE_INDEX, clause);
-      handleChange(nextQuery);
-    },
-    [query, handleChange],
-  );
-
-  const handleReplaceBreakouts = useCallback(
-    (column: Lib.ColumnMetadata) => {
-      const nextQuery = Lib.replaceBreakouts(query, STAGE_INDEX, column);
-      handleChange(nextQuery);
-    },
-    [query, handleChange],
-  );
-  return {
-    query,
-    stageIndex: STAGE_INDEX,
-    aggregations,
-    hasAggregations,
-    handleQueryChange,
-    handleAddBreakout,
-    handleUpdateBreakout,
-    handleRemoveBreakout,
-    handleReplaceBreakouts,
-  };
-};
-
-function shouldAddDefaultAggregation(query: Lib.Query): boolean {
-  const aggregations = Lib.aggregations(query, STAGE_INDEX);
-  return aggregations.length === 0;
-}

frontend/src/metabase/query_builder/components/view/sidebars/SummarizeSidebar/SummarizeSidebar.tsx

@@ -2,42 +2,37 @@ import { useCallback } from "react";
 import { t } from "ttag";
 
 import { color } from "metabase/lib/colors";
+import type { UpdateQueryHookProps } from "metabase/query_builder/hooks/types";
+import { useDefaultQueryAggregation } from "metabase/query_builder/hooks/use-default-query-aggregation";
 import { Divider } from "metabase/ui";
-import type * as Lib from "metabase-lib";
 
 import {
   SummarizeAggregationItemList,
   SummarizeBreakoutColumnList,
-  useSummarizeQuery,
 } from "./SummarizeContent";
 import { SidebarView } from "./SummarizeSidebar.styled";
 
-interface SummarizeSidebarProps {
+type SummarizeSidebarProps = {
   className?: string;
-  query: Lib.Query;
-  onQueryChange: (query: Lib.Query) => void;
   onClose: () => void;
-}
+} & UpdateQueryHookProps;
 
 export function SummarizeSidebar({
   className,
   query: initialQuery,
   onQueryChange,
   onClose,
+  stageIndex,
 }: SummarizeSidebarProps) {
   const {
     query,
-    stageIndex,
-    aggregations,
+    onUpdateQuery: onDefaultQueryChange,
+    onAggregationChange,
     hasAggregations,
-    handleQueryChange,
-    handleAddBreakout,
-    handleUpdateBreakout,
-    handleRemoveBreakout,
-    handleReplaceBreakouts,
-  } = useSummarizeQuery({
+  } = useDefaultQueryAggregation({
     query: initialQuery,
     onQueryChange,
+    stageIndex,
   });
 
   const handleDoneClick = useCallback(() => {
@@ -55,20 +50,16 @@ export function SummarizeSidebar({
       <SummarizeAggregationItemList
         px="lg"
         query={query}
+        onQueryChange={onAggregationChange}
         stageIndex={stageIndex}
-        aggregations={aggregations}
-        onQueryChange={handleQueryChange}
       />
       <Divider my="lg" />
       {hasAggregations && (
         <SummarizeBreakoutColumnList
           px="lg"
           query={query}
+          onQueryChange={onDefaultQueryChange}
           stageIndex={stageIndex}
-          onAddBreakout={handleAddBreakout}
-          onUpdateBreakout={handleUpdateBreakout}
-          onRemoveBreakout={handleRemoveBreakout}
-          onReplaceBreakouts={handleReplaceBreakouts}
         />
       )}
     </SidebarView>

frontend/src/metabase/query_builder/components/view/sidebars/SummarizeSidebar/SummarizeSidebar.unit.spec.tsx

@@ -72,6 +72,7 @@ async function setup({
           onQueryChange(nextQuery);
         }}
         onClose={onClose}
+        stageIndex={-1}
       />
     );
   }

frontend/src/metabase/query_builder/hooks/types.ts

+import type * as Lib from "metabase-lib";
+
+export type UpdateQueryHookProps = {
+  query: Lib.Query;
+  onQueryChange: (nextQuery: Lib.Query) => void;
+  stageIndex: number;
+};

frontend/src/metabase/query_builder/hooks/use-breakout-query-handlers.ts

+import { useCallback } from "react";
+
+import * as Lib from "metabase-lib";
+
+import type { UpdateQueryHookProps } from "./types";
+
+export const useBreakoutQueryHandlers = ({
+  query,
+  onQueryChange,
+  stageIndex,
+}: UpdateQueryHookProps) => {
+  const onAddBreakout = useCallback(
+    (column: Lib.ColumnMetadata) => {
+      const nextQuery = Lib.breakout(query, stageIndex, column);
+      onQueryChange(nextQuery);
+    },
+    [query, stageIndex, onQueryChange],
+  );
+
+  const onUpdateBreakout = useCallback(
+    (clause: Lib.BreakoutClause, column: Lib.ColumnMetadata) => {
+      const nextQuery = Lib.replaceClause(query, stageIndex, clause, column);
+      onQueryChange(nextQuery);
+    },
+    [query, stageIndex, onQueryChange],
+  );
+
+  const onRemoveBreakout = useCallback(
+    (clause: Lib.BreakoutClause) => {
+      const nextQuery = Lib.removeClause(query, stageIndex, clause);
+      onQueryChange(nextQuery);
+    },
+    [query, stageIndex, onQueryChange],
+  );
+
+  const onReplaceBreakouts = useCallback(
+    (column: Lib.ColumnMetadata) => {
+      const nextQuery = Lib.replaceBreakouts(query, stageIndex, column);
+      onQueryChange(nextQuery);
+    },
+    [query, stageIndex, onQueryChange],
+  );
+
+  return {
+    onAddBreakout,
+    onUpdateBreakout,
+    onRemoveBreakout,
+    onReplaceBreakouts,
+  };
+};

frontend/src/metabase/query_builder/hooks/use-default-query-aggregation.ts

+import { useCallback, useMemo, useState } from "react";
+
+import * as Lib from "metabase-lib";
+
+import type { UpdateQueryHookProps } from "./types";
+
+export const useDefaultQueryAggregation = ({
+  query: initialQuery,
+  onQueryChange,
+  stageIndex,
+}: UpdateQueryHookProps) => {
+  const [hasDefaultAggregation, setHasDefaultAggregation] = useState(() =>
+    shouldAddDefaultAggregation(initialQuery, stageIndex),
+  );
+
+  const query = useMemo(
+    () =>
+      hasDefaultAggregation
+        ? Lib.aggregateByCount(initialQuery, stageIndex)
+        : initialQuery,
+    [hasDefaultAggregation, initialQuery, stageIndex],
+  );
+
+  const hasAggregations = useMemo(
+    () => Lib.aggregations(query, stageIndex).length > 0,
+    [query, stageIndex],
+  );
+
+  const onUpdateQuery = useCallback(
+    (nextQuery: Lib.Query) => {
+      setHasDefaultAggregation(false);
+      onQueryChange(nextQuery);
+    },
+    [onQueryChange],
+  );
+
+  const onAggregationChange = useCallback(
+    (nextQuery: Lib.Query) => {
+      const newAggregations = Lib.aggregations(nextQuery, stageIndex);
+      setHasDefaultAggregation(false);
+
+      if (!hasDefaultAggregation || newAggregations.length !== 0) {
+        onQueryChange(nextQuery);
+      }
+    },
+    [hasDefaultAggregation, onQueryChange, stageIndex],
+  );
+
+  return {
+    query,
+    hasAggregations,
+    onUpdateQuery,
+    onAggregationChange,
+  };
+};
+
+function shouldAddDefaultAggregation(
+  query: Lib.Query,
+  stageIndex = -1,
+): boolean {
+  const aggregations = Lib.aggregations(query, stageIndex);
+  return aggregations.length === 0;
+}

frontend/src/metabase/query_builder/utils/get-aggregation-items.ts

+import * as Lib from "metabase-lib";
+
+import type { UpdateQueryHookProps } from "../hooks/types";
+
+export const getAggregationItems = ({
+  query,
+  stageIndex,
+}: Pick<UpdateQueryHookProps, "query" | "stageIndex">) => {
+  const aggregations = Lib.aggregations(query, stageIndex);
+
+  return aggregations.map((aggregation, aggregationIndex) => {
+    const { displayName } = Lib.displayInfo(query, stageIndex, aggregation);
+
+    const operators = Lib.selectedAggregationOperators(
+      Lib.availableAggregationOperators(query, stageIndex),
+      aggregation,
+    );
+
+    return {
+      aggregation,
+      aggregationIndex,
+      operators,
+      displayName,
+    };
+  });
+};