Merge branch 'master' into put-object-options

.github/workflows/deploy-lambdas.yaml

@@ -17,10 +17,8 @@ jobs:
           - indexer
           - pkgevents
           - pkgpush
-          - pkgselect
           - preview
           - s3hash
-          - s3select
           - status_reports
           - tabular_preview
           - transcode

.github/workflows/py-ci.yml

@@ -160,10 +160,8 @@ jobs:
           - molecule
           - pkgevents
           - pkgpush
-          - pkgselect
           - preview
           - s3hash
-          - s3select
           - shared
           - status_reports
           - tabular_preview

catalog/CHANGELOG.md

@@ -17,6 +17,8 @@ where verb is one of
 
 ## Changes
 
+- [Fixed] Athena: fix minor UI bugs ([#4232](https://github.com/quiltdata/quilt/pull/4232))
+- [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/Database.tsx

@@ -205,15 +205,8 @@ const useStyles = M.makeStyles((t) => ({
     display: 'flex',
   },
   field: {
-    cursor: 'pointer',
     flexBasis: '50%',
     marginRight: t.spacing(2),
-    '& input': {
-      cursor: 'pointer',
-    },
-    '& > *': {
-      cursor: 'pointer',
-    },
   },
   button: {
     marginLeft: t.spacing(1),

catalog/app/containers/Bucket/Queries/Athena/History.tsx

@@ -277,6 +277,13 @@ export default function History({ bucket, executions, onLoadMore }: HistoryProps
   const { workgroup } = Model.use()
   if (!Model.hasValue(workgroup)) return null
 
+  if (!executions.length)
+    return (
+      <M.Paper>
+        <Empty />
+      </M.Paper>
+    )
+
   return (
     <>
       <M.Paper>
@@ -304,7 +311,6 @@ export default function History({ bucket, executions, onLoadMore }: HistoryProps
             />
           ),
         )}
-        {!executions.length && <Empty />}
         {(hasPagination || !!onLoadMore) && (
           <div className={classes.footer}>
             {hasPagination && (

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])