Bug fix: show Athena query editor when no named quries (#4230)

Co-authored-by: Alexei Mochalov <[email protected]>

catalog/CHANGELOG.md

@@ -17,6 +17,7 @@ where verb is one of
 
 ## Changes
 
+- [Fixed] Show Athena query editor when no named queries ([#4230](https://github.com/quiltdata/quilt/pull/4230))
 - [Fixed] Fix some doc URLs in catalog ([#4205](https://github.com/quiltdata/quilt/pull/4205))
 - [Changed] S3 Select -> GQL API calls for getting access counts ([#4218](https://github.com/quiltdata/quilt/pull/4218))
 - [Changed] Athena: improve loading state and errors visuals; fix minor bugs; alphabetize and persist selection in workgroups, catalog names and databases ([#4208](https://github.com/quiltdata/quilt/pull/4208))

catalog/app/containers/Bucket/Queries/Athena/model/requests.spec.ts

@@ -852,6 +852,48 @@ describe('containers/Bucket/Queries/Athena/model/requests', () => {
         unmount()
       })
     })
+
+    it('return "not ready" if database is not ready', async () => {
+      startQueryExecution.mockImplementation(
+        reqThen<A.StartQueryExecutionInput, A.StartQueryExecutionOutput>(() => ({})),
+      )
+      await act(async () => {
+        const { result, unmount, waitForNextUpdate } = renderHook(() =>
+          requests.useQueryRun({
+            workgroup: 'a',
+            catalogName: 'b',
+            database: Model.Loading,
+            queryBody: 'd',
+          }),
+        )
+        await waitForNextUpdate()
+        expect(result.current[0]).toBeUndefined()
+        unmount()
+      })
+    })
+
+    it('mark as ready to run but return error for confirmation if database is empty', async () => {
+      startQueryExecution.mockImplementation(
+        reqThen<A.StartQueryExecutionInput, A.StartQueryExecutionOutput>(() => ({})),
+      )
+      await act(async () => {
+        const { result, unmount, waitForValueToChange } = renderHook(() =>
+          requests.useQueryRun({
+            workgroup: 'a',
+            catalogName: 'b',
+            database: '',
+            queryBody: 'd',
+          }),
+        )
+        await waitForValueToChange(() => result.current)
+        await waitForValueToChange(() => result.current[0])
+        expect(result.current[0]).toBeNull()
+        const run = await result.current[1](false)
+        expect(run).toBeInstanceOf(Error)
+        expect(run).toBe(requests.NO_DATABASE)
+        unmount()
+      })
+    })
   })
 
   describe('useWorkgroup', () => {
@@ -1036,7 +1078,7 @@ describe('containers/Bucket/Queries/Athena/model/requests', () => {
       }
     })
 
-    it('does not change query if a valid query is already selected', async () => {
+    it('retains execution query when the list is changed', async () => {
       const queries = {
         list: [
           { key: 'foo', name: 'Foo', body: 'SELECT * FROM foo' },
@@ -1063,8 +1105,7 @@ describe('containers/Bucket/Queries/Athena/model/requests', () => {
           {
             list: [
               { key: 'baz', name: 'Baz', body: 'SELECT * FROM baz' },
-              { key: 'foo', name: 'Foo', body: 'SELECT * FROM foo' },
-              { key: 'bar', name: 'Bar', body: 'SELECT * FROM bar' },
+              ...queries.list,
             ],
           },
           execution,
@@ -1077,6 +1118,45 @@ describe('containers/Bucket/Queries/Athena/model/requests', () => {
         throw new Error('No data')
       }
     })
+
+    it('does not change query when list is updated if a valid query is already selected', async () => {
+      const queries = {
+        list: [
+          { key: 'foo', name: 'Foo', body: 'SELECT * FROM foo' },
+          { key: 'bar', name: 'Bar', body: 'SELECT * FROM bar' },
+        ],
+      }
+      const execution = null
+      const { result, rerender, waitForNextUpdate } = renderHook(
+        (props: Parameters<typeof requests.useQuery>) => useWrapper(props),
+        {
+          initialProps: [queries, execution],
+        },
+      )
+
+      if (Model.hasData(result.current.value)) {
+        expect(result.current.value.body).toBe('SELECT * FROM foo')
+      } else {
+        throw new Error('No data')
+      }
+      await act(async () => {
+        rerender([
+          {
+            list: [
+              { key: 'baz', name: 'Baz', body: 'SELECT * FROM baz' },
+              ...queries.list,
+            ],
+          },
+          execution,
+        ])
+        await waitForNextUpdate()
+      })
+      if (Model.hasData(result.current.value)) {
+        expect(result.current.value.body).toBe('SELECT * FROM foo')
+      } else {
+        throw new Error('No data')
+      }
+    })
   })
 
   describe('useQueryBody', () => {
@@ -1086,7 +1166,7 @@ describe('containers/Bucket/Queries/Athena/model/requests', () => {
 
     it('sets query body from query if query is ready', () => {
       const query = { name: 'Foo', key: 'foo', body: 'SELECT * FROM foo' }
-      const execution = {}
+      const execution = null
       const setQuery = jest.fn()
 
       const { result } = renderHook(() => useWrapper([query, setQuery, execution]))
@@ -1098,7 +1178,7 @@ describe('containers/Bucket/Queries/Athena/model/requests', () => {
       }
     })
 
-    it('sets query body from execution if query is not ready', () => {
+    it('sets query body from execution if query is not selected', () => {
       const query = null
       const execution = { query: 'SELECT * FROM bar' }
       const setQuery = jest.fn()
@@ -1127,8 +1207,8 @@ describe('containers/Bucket/Queries/Athena/model/requests', () => {
     })
 
     it('does not change value if query and execution are both not ready', async () => {
-      const query = null
-      const execution = null
+      const query = undefined
+      const execution = undefined
       const setQuery = jest.fn()
 
       const { result, rerender, waitForNextUpdate } = renderHook(
@@ -1139,11 +1219,15 @@ describe('containers/Bucket/Queries/Athena/model/requests', () => {
       )
 
       expect(result.current.value).toBeUndefined()
+      // That's not possible from UI now,
+      // but let's pretend UI is ready to handle user input
       act(() => {
         result.current.setValue('foo')
       })
       expect(result.current.value).toBe('foo')
 
+      // We rerenderd hook but internal useEffect didn't rewrite the value
+      // to `undefined` as it was supposed to do on the first render
       await act(async () => {
         rerender([query, setQuery, execution])
         await waitForNextUpdate()
@@ -1166,7 +1250,7 @@ describe('containers/Bucket/Queries/Athena/model/requests', () => {
       expect(setQuery).toHaveBeenCalledWith(null)
     })
 
-    it('retains value when execution and query are initially empty but later updates', async () => {
+    it('obtains value when execution and query are initially empty but later update', async () => {
       const initialQuery = null
       const initialExecution = null
       const setQuery = jest.fn()
@@ -1178,8 +1262,10 @@ describe('containers/Bucket/Queries/Athena/model/requests', () => {
         },
       )
 
-      expect(result.current.value).toBeUndefined()
+      expect(result.current.value).toBeNull()
 
+      // Query was loaded with some value
+      // Execution is ready but it's still null
       await act(async () => {
         rerender([
           { key: 'up', name: 'Updated', body: 'SELECT * FROM updated' },
@@ -1195,5 +1281,68 @@ describe('containers/Bucket/Queries/Athena/model/requests', () => {
         throw new Error('No data')
       }
     })
+
+    it('sets query body to null if query is null after being loaded', async () => {
+      const initialQuery = Model.Loading
+      const initialExecution = null
+      const setQuery = jest.fn()
+
+      const { result, rerender, waitForNextUpdate } = renderHook(
+        (props: Parameters<typeof requests.useQueryBody>) => useWrapper(props),
+        {
+          initialProps: [
+            initialQuery as Model.Value<requests.Query>,
+            setQuery,
+            initialExecution,
+          ],
+        },
+      )
+
+      expect(result.current.value).toBe(Model.Loading)
+
+      await act(async () => {
+        rerender([null, setQuery, initialExecution])
+        await waitForNextUpdate()
+      })
+
+      if (Model.hasValue(result.current.value)) {
+        expect(result.current.value).toBeNull()
+      } else {
+        throw new Error('Unexpected state')
+      }
+    })
+
+    it('retains value if selected query is null and we switch from some execution', async () => {
+      // That's not ideal,
+      // but we don't know what chanded the query body: execution page or user.
+      // So, at least, it is documented here.
+      const initialQuery = null
+      const initialExecution = { id: 'any', query: 'SELECT * FROM updated' }
+      const setQuery = jest.fn()
+
+      const { result, rerender, waitForNextUpdate } = renderHook(
+        (props: Parameters<typeof requests.useQueryBody>) => useWrapper(props),
+        {
+          initialProps: [
+            initialQuery as Model.Value<requests.Query>,
+            setQuery,
+            initialExecution,
+          ],
+        },
+      )
+
+      expect(result.current.value).toBe('SELECT * FROM updated')
+
+      await act(async () => {
+        rerender([initialQuery, setQuery, null])
+        await waitForNextUpdate()
+      })
+
+      if (Model.hasValue(result.current.value)) {
+        expect(result.current.value).toBe('SELECT * FROM updated')
+      } else {
+        throw new Error('Unexpected state')
+      }
+    })
   })
 })

catalog/app/containers/Bucket/Queries/Athena/model/requests.ts

@@ -630,6 +630,9 @@ export function useQueryBody(
       if (Model.isError(query)) return null
       if (Model.hasData(query)) return query.body
       if (Model.hasData(execution) && execution.query) return execution.query
+      if (!Model.isReady(v) && Model.isReady(query) && Model.isReady(execution)) {
+        return null
+      }
       return v
     })
   }, [execution, query])