From 21abbf64ba41b5cafe55cdae1cbcc5ff5d4c19e0 Mon Sep 17 00:00:00 2001
From: James Kerr <kerr@hey.com>
Date: Mon, 10 Jun 2024 10:38:57 -0700
Subject: [PATCH] Parse Error UI Improvement (#3085)

* Handle Error Case Differently
* Handle missing properties in query info
* Do not show the histogram if there is a parse error
* Loosen missing pool error detection
* Improve histogram error message
---
 apps/zui/src/js/state/QueryInfo/selectors.ts        | 13 +++++++++----
 apps/zui/src/models/browser-tab.ts                  |  1 -
 apps/zui/src/views/histogram-pane/index.tsx         |  4 +++-
 apps/zui/src/views/histogram-pane/run-query.ts      |  4 +++-
 .../results-pane/errors/missing-pool-error.tsx      |  2 +-
 apps/zui/src/views/session-page/handler.ts          |  6 ++----
 6 files changed, 18 insertions(+), 12 deletions(-)

diff --git a/apps/zui/src/js/state/QueryInfo/selectors.ts b/apps/zui/src/js/state/QueryInfo/selectors.ts
index f0abb1d850..d7028963a9 100644
--- a/apps/zui/src/js/state/QueryInfo/selectors.ts
+++ b/apps/zui/src/js/state/QueryInfo/selectors.ts
@@ -9,12 +9,17 @@ export const get = activeTabSelect((tab) => {
 export const getParseError = createSelector(get, (info) => info.error)
 export const getIsParsed = createSelector(get, (info) => info.isParsed)
 export const getPoolName = createSelector(get, (info) => {
-  let source = find(info.sources, {kind: "Pool"})
+  let source = find(info.sources || [], {kind: "Pool"})
   return source ? source.name : null
 })
 export const getGroupByKeys = createSelector(get, (info) => {
-  return info.channels[0].aggregation_keys
+  const {channels} = info
+  if (channels) {
+    return channels[0].aggregation_keys || []
+  } else {
+    return []
+  }
 })
-export const hasAggregation = createSelector(get, (info) => {
-  return !!info.channels[0].aggregation_keys
+export const hasAggregation = createSelector(getGroupByKeys, (keys) => {
+  return keys.length > 0
 })
diff --git a/apps/zui/src/models/browser-tab.ts b/apps/zui/src/models/browser-tab.ts
index 5be77fd1dd..f6b9797f64 100644
--- a/apps/zui/src/models/browser-tab.ts
+++ b/apps/zui/src/models/browser-tab.ts
@@ -49,7 +49,6 @@ export class BrowserTab extends DomainModel<Attrs> {
         this.history.index = -1
       }
       this.history.push(pathname)
-      console.log(this.history)
     }
   }
 
diff --git a/apps/zui/src/views/histogram-pane/index.tsx b/apps/zui/src/views/histogram-pane/index.tsx
index df89cede7d..de8db66cd6 100644
--- a/apps/zui/src/views/histogram-pane/index.tsx
+++ b/apps/zui/src/views/histogram-pane/index.tsx
@@ -8,13 +8,15 @@ import {Toolbar} from "src/components/toolbar"
 import {Title} from "./title"
 import {Resizer} from "./resizer"
 import {useRef} from "react"
+import QueryInfo from "src/js/state/QueryInfo"
 
 export function HistogramPane() {
   const {Parent, width = 0, height = 0} = useParentSize()
   const show = useSelector(Layout.getShowHistogram)
   const chartHeight = useSelector(Layout.getChartHeight)
+  const parseError = useSelector(QueryInfo.getParseError)
   const ref = useRef<HTMLDivElement>()
-  if (!show) return null
+  if (!show || parseError) return null
 
   return (
     <div
diff --git a/apps/zui/src/views/histogram-pane/run-query.ts b/apps/zui/src/views/histogram-pane/run-query.ts
index 54dd85afbd..4cc212434a 100644
--- a/apps/zui/src/views/histogram-pane/run-query.ts
+++ b/apps/zui/src/views/histogram-pane/run-query.ts
@@ -40,7 +40,9 @@ export const runHistogramQuery = createHandler(
       async function getPoolRange() {
         const queryText = `from ${poolId} | min(${timeField}), max(${timeField})`
         const resp = await query(queryText, {signal})
-        const [{min, max}] = await resp.js()
+        const data = await resp.js()
+        if (data.length == 0) return null
+        const [{min, max}] = data
         if (!(min instanceof Date && max instanceof Date)) return null
         return [min, max] as [Date, Date]
       }
diff --git a/apps/zui/src/views/results-pane/errors/missing-pool-error.tsx b/apps/zui/src/views/results-pane/errors/missing-pool-error.tsx
index cd9b01f345..afaadf97ee 100644
--- a/apps/zui/src/views/results-pane/errors/missing-pool-error.tsx
+++ b/apps/zui/src/views/results-pane/errors/missing-pool-error.tsx
@@ -44,7 +44,7 @@ const Card = styled.section`
 `
 
 export function isMissingPoolError(e: unknown) {
-  return e === "no pool name given"
+  return typeof e === "string" && /no pool name given/.test(e)
 }
 
 function PoolsList({pools}: {pools: Pool[]}) {
diff --git a/apps/zui/src/views/session-page/handler.ts b/apps/zui/src/views/session-page/handler.ts
index d02f1ae7b8..66d48803df 100644
--- a/apps/zui/src/views/session-page/handler.ts
+++ b/apps/zui/src/views/session-page/handler.ts
@@ -66,18 +66,16 @@ export class SessionPageHandler extends ViewHandler {
     const history = this.select(Current.getHistory)
 
     fetchQueryInfo(program).then((info) => {
-      if (info.error) {
-        return
-      }
       this.dispatch(QueryInfo.set({isParsed: true, ...info}))
       const poolName = this.select(QueryInfo.getPoolName)
       this.invoke("updatePluginSessionOp", {poolName, program})
       const pool = this.select(Pools.getByName(lakeId, poolName))
+
       if (pool && !pool.hasSpan()) {
         this.dispatch(syncPool(pool.id, lakeId))
       }
 
-      if (history.action === "PUSH") {
+      if (!info.error && history.action === "PUSH") {
         session.pushHistory()
       }
     })