Catalog: honor a mis-cased Athena workgroup and stop masking the bucket default (5217 stack 7/9)

catalog/CHANGELOG.md

@@ -21,6 +21,7 @@ complete sentence without it.
 
 ## Changes
 
+- [Fixed] Athena Queries: a workgroup named in the URL is honored whatever its capitalisation, instead of matching a workgroup you can see and then having every AWS call rejected ([#5265](https://github.com/quiltdata/quilt/pull/5265))
 - [Fixed] Admin Users: the roles dialog and the Role column are read-only for service users, rather than offering a Save the registry refuses ([#5264](https://github.com/quiltdata/quilt/pull/5264))
 - [Fixed] Admin Users: a disabled Enabled or Admin switch says why on hover and on keyboard focus — your own account, a service user managed by the stack, or admin capabilities managed by the SSO configuration — instead of rendering as the same unexplained dead control; the Admin switch now also refuses service users ([#5263](https://github.com/quiltdata/quilt/pull/5263))
 - [Fixed] Search sidebar: the facet "Sort by" control no longer disappears while you type in "Find metadata" on stacks with truncated facet lists, and it is withheld when the query matches nothing rather than offering to sort an empty list ([#5262](https://github.com/quiltdata/quilt/pull/5262))

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

@@ -1266,10 +1266,9 @@ describe('containers/Queries/Athena/model/requests', () => {
         data: { list: ['foo', 'bar'] },
         loadMore: noop,
       }
-      const preferences = { defaultWorkgroup: 'bar' }
 
       const { result, waitFor, unmount } = renderHook(() =>
-        useWrapper([workgroups, undefined, preferences]),
+        useWrapper([workgroups, undefined, 'bar']),
       )
 
       await act(async () => {
@@ -1279,6 +1278,128 @@ describe('containers/Queries/Athena/model/requests', () => {
       unmount()
     })
 
+    it('stores the workgroup as AWS spells it, not as the caller did', async () => {
+      const workgroups = {
+        data: { list: ['foo', 'analytics-prod'] },
+        loadMore: noop,
+      }
+
+      const { result, waitFor, unmount } = renderHook(() =>
+        useWrapper([workgroups, undefined, 'Analytics-Prod']),
+      )
+
+      await act(async () => {
+        await waitFor(() => typeof result.current.data === 'string')
+        expect(result.current.data).toBe('analytics-prod')
+      })
+      unmount()
+    })
+
+    it('prefers an exact match over a differently-cased sibling', async () => {
+      // Workgroup names are case-sensitive, so both can exist. `fetchWorkgroups`
+      // sorts, which puts 'Alpha' first -- a case-insensitive match alone would
+      // hand the user the workgroup they did not ask for.
+      const workgroups = {
+        data: { list: ['Alpha', 'alpha'] },
+        loadMore: noop,
+      }
+
+      const { result, waitFor, unmount } = renderHook(() =>
+        useWrapper([workgroups, 'alpha', undefined]),
+      )
+
+      await act(async () => {
+        await waitFor(() => typeof result.current.data === 'string')
+        expect(result.current.data).toBe('alpha')
+      })
+      unmount()
+    })
+
+    it('falls back rather than throwing when the default is not a string', async () => {
+      // `ui.athena` is unconstrained by the bucket-config schema, so an all-digit
+      // workgroup name in the YAML arrives as a number. Reaching `toLowerCase`
+      // with it threw out of the effect and took the console down.
+      const workgroups = {
+        data: { list: ['foo', '2024'] },
+        loadMore: noop,
+      }
+
+      const { result, waitFor, unmount } = renderHook(() =>
+        useWrapper([workgroups, undefined, 2024 as unknown as string]),
+      )
+
+      await act(async () => {
+        await waitFor(() => typeof result.current.data === 'string')
+        expect(result.current.data).toBe('foo')
+      })
+      unmount()
+    })
+
+    it('canonicalises a mis-cased workgroup from the URL too', async () => {
+      const workgroups = {
+        data: { list: ['foo', 'analytics-prod'] },
+        loadMore: noop,
+      }
+
+      const { result, waitFor, unmount } = renderHook(() =>
+        useWrapper([workgroups, 'ANALYTICS-PROD', undefined]),
+      )
+
+      await act(async () => {
+        await waitFor(() => typeof result.current.data === 'string')
+        expect(result.current.data).toBe('analytics-prod')
+      })
+      unmount()
+    })
+
+    it('falls through to the bucket default when the stored workgroup is gone', async () => {
+      const storageMock = getStorageKey.getMockImplementation()
+      getStorageKey.mockImplementation(() => 'deleted')
+      const workgroups = {
+        data: { list: ['alpha', 'team'] },
+        loadMore: noop,
+      }
+
+      try {
+        const { result, waitFor, unmount } = renderHook(() =>
+          useWrapper([workgroups, undefined, 'team']),
+        )
+
+        await act(async () => {
+          await waitFor(() => typeof result.current.data === 'string')
+          expect(result.current.data).toBe('team')
+        })
+        unmount()
+      } finally {
+        getStorageKey.mockImplementation(storageMock!)
+      }
+    })
+
+    it('keeps the stored workgroup ahead of the bucket default', async () => {
+      // Precedence pinned, not chosen here: storage-before-default predates this
+      // code path, so changing it is a product decision.
+      const storageMock = getStorageKey.getMockImplementation()
+      getStorageKey.mockImplementation(() => 'alpha')
+      const workgroups = {
+        data: { list: ['alpha', 'team'] },
+        loadMore: noop,
+      }
+
+      try {
+        const { result, waitFor, unmount } = renderHook(() =>
+          useWrapper([workgroups, undefined, 'team']),
+        )
+
+        await act(async () => {
+          await waitFor(() => typeof result.current.data === 'string')
+          expect(result.current.data).toBe('alpha')
+        })
+        unmount()
+      } finally {
+        getStorageKey.mockImplementation(storageMock!)
+      }
+    })
+
     it('select the first available workgroup if no requested or default', async () => {
       await act(async () => {
         const workgroups = {

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

@@ -4,7 +4,6 @@ import * as React from 'react'
 import * as Sentry from '@sentry/react'
 
 import * as AWS from 'utils/AWS'
-import * as BucketPreferences from 'utils/BucketPreferences'
 import Log from 'utils/Logging'
 import noop from 'utils/noop'
 
@@ -29,8 +28,24 @@ function parseNamedQuery(query: Athena.NamedQuery): Query {
   }
 }
 
+/**
+ * The list's own spelling of `value`, preferring an exact match.
+ *
+ * Callers must use the spelling this returns: AWS calls send the workgroup name
+ * verbatim and reject a mis-cased one with `InvalidRequestException`. Names are
+ * case-sensitive, so `alpha` and `Alpha` can both exist -- an exact hit has to
+ * win over the case-insensitive one, which the sorted list would otherwise
+ * resolve to whichever sorts first.
+ */
+function canonical(list: string[], value: string): string | undefined {
+  return (
+    list.find((x) => x === value) ??
+    list.find((x) => x.toLowerCase() === value.toLowerCase())
+  )
+}
+
 function listIncludes(list: string[], value: string): boolean {
-  return list.map((x) => x.toLowerCase()).includes(value.toLowerCase())
+  return canonical(list, value) !== undefined
 }
 
 export type Workgroup = string
@@ -190,29 +205,34 @@ export function useWorkgroups(): Model.DataController<Model.List<Workgroup>> {
 export function useWorkgroup(
   workgroups: Model.DataController<Model.List<Workgroup>>,
   requestedWorkgroup?: Workgroup,
-  preferences?: BucketPreferences.AthenaPreferences,
+  // A string rather than the `ui.athena` object: the parsed preferences are
+  // rebuilt on every provider render, so an object would re-fire this effect
+  // throughout the workgroup probe.
+  defaultWorkgroup?: string,
 ): Model.DataController<Workgroup> {
   const [data, setData] = React.useState<Model.Data<Workgroup>>()
   React.useEffect(() => {
     if (!Model.hasData(workgroups.data)) return
-
-    // URL parameter workgroup (user navigation)
-    if (requestedWorkgroup && listIncludes(workgroups.data.list, requestedWorkgroup)) {
-      setData(requestedWorkgroup)
-      return
+    const { list } = workgroups.data
+
+    // `unknown`, not `string`: `defaultWorkgroup` comes from a user-authored YAML
+    // document, and the bucket-config schema does not constrain `ui.athena`.
+    const pick = (candidate: unknown): boolean => {
+      if (typeof candidate !== 'string' || !candidate) return false
+      const found = canonical(list, candidate)
+      if (!found) return false
+      setData(found)
+      return true
     }
 
-    // Stored or default workgroup
-    const initialWorkgroup = storage.getWorkgroup() || preferences?.defaultWorkgroup
-    if (initialWorkgroup && listIncludes(workgroups.data.list, initialWorkgroup)) {
-      setData(initialWorkgroup)
-      return
-    }
+    if (pick(requestedWorkgroup)) return
+    if (pick(storage.getWorkgroup())) return
+    if (pick(defaultWorkgroup)) return
 
     // First available workgroup or error. Producer drains to exhaustion, so
     // an accessible workgroup that exists is in this list.
-    setData(workgroups.data.list[0] || new Error('Workgroup not found'))
-  }, [preferences, requestedWorkgroup, workgroups])
+    setData(list[0] || new Error('Workgroup not found'))
+  }, [defaultWorkgroup, requestedWorkgroup, workgroups])
   return React.useMemo(() => ({ data, loadMore: noop }), [data])
 }
 

catalog/app/containers/Queries/Athena/model/state.spec.tsx

@@ -102,4 +102,56 @@ describe('app/containers/Queries/Athena/model/state', () => {
     expect(result.current.workgroup.data).toBe('w')
     unmount()
   })
+
+  it('threads the bucket default workgroup through to the model', async () => {
+    // The workgroup named in the URL is deliberately one this user cannot reach:
+    // without a named workgroup the provider redirects instead of rendering, so
+    // this is the only shape in which the default can be observed.
+    useParams.mockImplementation(() => ({ workgroup: 'gone' }) as Record<string, string>)
+    listWorkGroups.mockImplementation(() => ({
+      promise: () =>
+        Promise.resolve({
+          WorkGroups: [{ Name: 'alpha' }, { Name: 'team' }],
+        }),
+    }))
+    getWorkGroup.mockImplementation(({ WorkGroup: Name }: { WorkGroup: string }) => ({
+      promise: () =>
+        Promise.resolve({
+          WorkGroup: {
+            Configuration: { ResultConfiguration: { OutputLocation: 'any' } },
+            State: 'ENABLED',
+            Name,
+          },
+        }),
+    }))
+    listNamedQueries.mockImplementation((_x, cb) => {
+      cb(undefined, { NamedQueryIds: [] })
+      return { abort: noop }
+    })
+    listQueryExecutions.mockImplementation((_x, cb) => {
+      cb(undefined, { QueryExecutionIds: [] })
+      return { abort: noop }
+    })
+    listDataCatalogs.mockImplementation(() => ({
+      promise: () => Promise.resolve({ DataCatalogsSummary: [] }),
+    }))
+
+    const wrapper = ({ children }: { children: React.ReactNode }) => (
+      <Model.Provider preferences={{ defaultWorkgroup: 'team' }}>
+        {children}
+      </Model.Provider>
+    )
+    try {
+      const { result, waitFor, unmount } = renderHook(() => Model.useState(), { wrapper })
+      await act(async () => {
+        await waitFor(() => typeof result.current.workgroup.data === 'string')
+      })
+      // 'alpha' is first in the list, so this is the default being honored rather
+      // than the fallback happening to agree.
+      expect(result.current.workgroup.data).toBe('team')
+      unmount()
+    } finally {
+      useParams.mockImplementation(() => ({ workgroup: 'w' }) as Record<string, string>)
+    }
+  })
 })

catalog/app/containers/Queries/Athena/model/state.tsx

@@ -2,6 +2,7 @@ import invariant from 'invariant'
 import * as React from 'react'
 import * as RRDom from 'react-router-dom'
 
+import type * as BucketPreferences from 'utils/BucketPreferences'
 import * as NamedRoutes from 'utils/NamedRoutes'
 
 import * as requests from './requests'
@@ -62,10 +63,15 @@ export interface State {
 export const Ctx = React.createContext<State | null>(null)
 
 interface ProviderProps {
+  /**
+   * `ui.athena` from the `?bucket=` scope's preferences. The console is
+   * workspace-global, so this only arrives when a bucket is in scope.
+   */
+  preferences?: BucketPreferences.AthenaPreferences
   children: React.ReactNode
 }
 
-export function Provider({ children }: ProviderProps) {
+export function Provider({ preferences, children }: ProviderProps) {
   const { urls } = NamedRoutes.use()
   const location = RRDom.useLocation()
 
@@ -77,7 +83,11 @@ export function Provider({ children }: ProviderProps) {
   const execution = requests.useWaitForQueryExecution(queryExecutionId)
 
   const workgroups = requests.useWorkgroups()
-  const workgroup = requests.useWorkgroup(workgroups, workgroupId)
+  const workgroup = requests.useWorkgroup(
+    workgroups,
+    workgroupId,
+    preferences?.defaultWorkgroup,
+  )
   const queries = requests.useQueries(workgroup.data)
   const query = requests.useQuery(queries.data, execution)
   const queryBody = requests.useQueryBody(query.value, query.setValue, execution)