diff --git a/catalog/CHANGELOG.md b/catalog/CHANGELOG.md index ab6c3f62946..b6d09c52751 100644 --- a/catalog/CHANGELOG.md +++ b/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)) diff --git a/catalog/app/containers/Bucket/Queries/Athena/model/requests.spec.ts b/catalog/app/containers/Bucket/Queries/Athena/model/requests.spec.ts index bf8a0793922..4faf6dbbcee 100644 --- a/catalog/app/containers/Bucket/Queries/Athena/model/requests.spec.ts +++ b/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(() => ({})), + ) + 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(() => ({})), + ) + 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) => 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) => useWrapper(props), + { + initialProps: [ + initialQuery as Model.Value, + 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) => useWrapper(props), + { + initialProps: [ + initialQuery as Model.Value, + 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') + } + }) }) }) diff --git a/catalog/app/containers/Bucket/Queries/Athena/model/requests.ts b/catalog/app/containers/Bucket/Queries/Athena/model/requests.ts index 227e6ecbde2..8da2450012b 100644 --- a/catalog/app/containers/Bucket/Queries/Athena/model/requests.ts +++ b/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])