Hide and opt-in file and directory delete buttons (#4689)

Author: fiskus

catalog/CHANGELOG.md

@@ -18,6 +18,7 @@ where verb is one of
 
 ## Changes
 
+- [Changed] Hide file and directory delete buttons in Bucket tab by default; enable with `ui.actions.deleteObject` ([#4689](https://github.com/quiltdata/quilt/pull/4689))
 - [Added] Display detailed permission error messages when bucket add/update fails due to insufficient S3 permissions ([#4670](https://github.com/quiltdata/quilt/pull/4670))
 - [Added] Support `prefixes` field in bucket configuration for prefix-scoped bucket access ([#4670](https://github.com/quiltdata/quilt/pull/4670))
 - [Fixed] Catch error and don't show Perspective table if Perspective failed to render ([#4669](https://github.com/quiltdata/quilt/pull/4669))

catalog/app/containers/Bucket/Dir/Toolbar/Get/Options.spec.tsx

@@ -32,14 +32,21 @@ describe('containers/Bucket/Dir/Toolbar/Get/Options', () => {
   }
 
   it('should render CodeSamples components with correct dest for nested path', () => {
-    render(<Options handle={DirHandleCreate('test-bucket', 'folder/subfolder/')} />)
+    render(
+      <Options
+        handle={DirHandleCreate('test-bucket', 'folder/subfolder/')}
+        features={{ code: true }}
+      />,
+    )
 
     expect(Quilt3Fetch).toHaveBeenCalledWith({ ...props, dest: 'subfolder' }, {})
     expect(CliFetch).toHaveBeenCalledWith({ ...props, dest: 'subfolder' }, {})
   })
 
   it('should render CodeSamples components with bucket as dest for root path', () => {
-    render(<Options handle={DirHandleCreate('test-bucket', '')} />)
+    render(
+      <Options handle={DirHandleCreate('test-bucket', '')} features={{ code: true }} />,
+    )
 
     expect(Quilt3Fetch).toHaveBeenCalledWith({ ...props, dest: 'test-bucket' }, {})
     expect(CliFetch).toHaveBeenCalledWith({ ...props, dest: 'test-bucket' }, {})

catalog/app/containers/Bucket/Dir/Toolbar/Get/Options.tsx

@@ -7,6 +7,7 @@ import * as CodeSamples from 'containers/Bucket/CodeSamples'
 import * as Buttons from 'containers/Bucket/Download/Buttons'
 import GetOptions from 'containers/Bucket/Toolbar/GetOptions'
 import type * as Toolbar from 'containers/Bucket/Toolbar'
+import type { Features } from '../useFeatures'
 
 const useCodeSamplesStyles = M.makeStyles((t) => ({
   code: {
@@ -61,14 +62,14 @@ function DownloadDir({ dirHandle }: DownloadDirProps) {
 
 interface OptionsProps {
   handle: Toolbar.DirHandle
-  hideCode?: boolean
+  features: Exclude<Features['get'], false>
 }
 
-export default function Options({ handle, hideCode }: OptionsProps) {
+export default function Options({ handle, features }: OptionsProps) {
   const download = <DownloadDir dirHandle={handle} />
-  const code = hideCode ? undefined : (
+  const code = features.code ? (
     <DirCodeSamples bucket={handle.bucket} path={handle.path} />
-  )
+  ) : undefined
 
   return <GetOptions download={download} code={code} />
 }

catalog/app/containers/Bucket/Dir/Toolbar/Organize/Options.tsx

@@ -5,6 +5,7 @@ import * as Icons from '@material-ui/icons'
 import * as Format from 'utils/format'
 import assertNever from 'utils/assertNever'
 
+import type { Features } from '../useFeatures'
 import * as Context from './Context'
 
 const LIST_ITEM_TYPOGRAPHY_PROPS = { noWrap: true }
@@ -34,7 +35,11 @@ const useStyles = M.makeStyles((t) => ({
   },
 }))
 
-export default function OrganizeOptions() {
+interface OrganizeOptionsProps {
+  features: Exclude<Features['organize'], false>
+}
+
+export default function OrganizeOptions({ features }: OrganizeOptionsProps) {
   const classes = useStyles()
   const {
     toggleBookmarks,
@@ -102,16 +107,20 @@ export default function OrganizeOptions() {
         />
       </M.List>
 
-      <M.Divider />
-
-      <M.List dense>
-        <MenuItem
-          className={classes.error}
-          icon={<Icons.DeleteOutlined color="error" />}
-          onClick={confirmDeleteSelected}
-          primary="Delete selected items"
-        />
-      </M.List>
+      {features.delete && (
+        <>
+          <M.Divider />
+
+          <M.List dense>
+            <MenuItem
+              className={classes.error}
+              icon={<Icons.DeleteOutlined color="error" />}
+              onClick={confirmDeleteSelected}
+              primary="Delete selected items"
+            />
+          </M.List>
+        </>
+      )}
     </>
   )
 }

catalog/app/containers/Bucket/Dir/Toolbar/Toolbar.spec.tsx

@@ -1,10 +1,8 @@
 import * as React from 'react'
-import { beforeEach, describe, it, expect, vi, afterEach, type Mock } from 'vitest'
+import { describe, it, expect, vi, afterEach, type Mock } from 'vitest'
 import { render, cleanup } from '@testing-library/react'
-import { renderHook } from '@testing-library/react-hooks'
 
 import * as BucketPreferences from 'utils/BucketPreferences'
-import { extendDefaults } from 'utils/BucketPreferences/BucketPreferences'
 import noop from 'utils/noop'
 
 import * as DirToolbar from './Toolbar'
@@ -19,14 +17,24 @@ vi.mock('./Add', () => ({
 }))
 
 vi.mock('./Get', () => ({
-  Options: () => <div>"Get" popover</div>,
+  Options: ({ features }: { features?: { code: boolean } }) => (
+    <div>
+      "Get" popover
+      {features?.code && <div data-testid="code-samples" />}
+    </div>
+  ),
 }))
 
 vi.mock('./Organize', () => ({
   Context: {
     Provider: ({ children }: { children: React.ReactNode }) => <>{children}</>,
   },
-  Options: () => <>"Organize" popover</>,
+  Options: ({ features }: { features?: { delete: boolean } }) => (
+    <div>
+      "Organize" popover
+      {features?.delete && <div data-testid="delete-button" />}
+    </div>
+  ),
 }))
 
 vi.mock('./CreatePackage', () => ({
@@ -70,78 +78,6 @@ vi.mock('utils/BucketPreferences', async () => ({
   use: () => prefsHook(),
 }))
 
-describe('useFeatures', () => {
-  beforeEach(() => {
-    vi.clearAllMocks()
-  })
-
-  it('should return null when preferences are loading', () => {
-    prefsHook.mockImplementationOnce(() => ({
-      prefs: BucketPreferences.Result.Pending(),
-    }))
-
-    const { result } = renderHook(() => DirToolbar.useFeatures())
-
-    expect(result.current).toBeNull()
-  })
-
-  it('should return all features disabled when all permissions are false', () => {
-    prefsHook.mockImplementationOnce(() => ({
-      prefs: BucketPreferences.Result.Ok(
-        extendDefaults({
-          ui: {
-            actions: {
-              writeFile: false,
-              downloadObject: false,
-              createPackage: false,
-            },
-            blocks: {
-              code: false,
-            },
-          },
-        }),
-      ),
-    }))
-
-    const { result } = renderHook(() => DirToolbar.useFeatures())
-
-    expect(result.current).toEqual({
-      add: false,
-      get: false,
-      organize: true, // Always true in the implementation
-      createPackage: false,
-    })
-  })
-
-  it('should return all features enabled when all permissions are true', () => {
-    prefsHook.mockImplementationOnce(() => ({
-      prefs: BucketPreferences.Result.Ok(
-        extendDefaults({
-          ui: {
-            actions: {
-              writeFile: true,
-              downloadObject: true,
-              createPackage: true,
-            },
-            blocks: {
-              code: true,
-            },
-          },
-        }),
-      ),
-    }))
-
-    const { result } = renderHook(() => DirToolbar.useFeatures())
-
-    expect(result.current).toEqual({
-      add: true,
-      get: { code: true },
-      organize: true,
-      createPackage: true,
-    })
-  })
-})
-
 const handle = DirToolbar.CreateHandle('test-bucket', 'test/path')
 
 describe('Toolbar', () => {
@@ -158,7 +94,12 @@ describe('Toolbar', () => {
   it('should render all buttons when all features are enabled', () => {
     const { getByTitle } = render(
       <DirToolbar.Toolbar
-        features={{ add: true, get: { code: true }, organize: true, createPackage: true }}
+        features={{
+          add: true,
+          get: { code: true },
+          organize: { delete: true },
+          createPackage: true,
+        }}
         handle={handle}
         onReload={vi.fn()}
       />,
@@ -186,7 +127,12 @@ describe('Toolbar', () => {
   it('should render buttons for enabled features: add, organize', () => {
     const { getByTitle, queryByTitle } = render(
       <DirToolbar.Toolbar
-        features={{ add: true, get: false, organize: true, createPackage: false }}
+        features={{
+          add: true,
+          get: false,
+          organize: { delete: true },
+          createPackage: false,
+        }}
         handle={handle}
         onReload={vi.fn()}
       />,
@@ -196,4 +142,36 @@ describe('Toolbar', () => {
     expect(queryByTitle('Get files')).toBeFalsy()
     expect(queryByTitle('Create package')).toBeFalsy()
   })
+
+  it('should show delete button in Organize.Options when delete feature is enabled', () => {
+    const { getByTestId } = render(
+      <DirToolbar.Toolbar
+        features={{
+          add: false,
+          get: false,
+          organize: { delete: true },
+          createPackage: false,
+        }}
+        handle={handle}
+        onReload={vi.fn()}
+      />,
+    )
+    expect(getByTestId('delete-button')).toBeTruthy()
+  })
+
+  it('should hide delete button in Organize.Options when delete feature is disabled', () => {
+    const { queryByTestId } = render(
+      <DirToolbar.Toolbar
+        features={{
+          add: false,
+          get: false,
+          organize: { delete: false },
+          createPackage: false,
+        }}
+        handle={handle}
+        onReload={vi.fn()}
+      />,
+    )
+    expect(queryByTestId('delete-button')).toBeFalsy()
+  })
 })

catalog/app/containers/Bucket/Dir/Toolbar/Toolbar.tsx

@@ -3,9 +3,6 @@ import * as React from 'react'
 import * as M from '@material-ui/core'
 import * as Lab from '@material-ui/lab'
 
-import cfg from 'constants/config'
-import * as BucketPreferences from 'utils/BucketPreferences'
-
 import * as Toolbar from 'containers/Bucket/Toolbar'
 import { FromHandles, useCreateDialog } from 'containers/Bucket/PackageDialog'
 import * as Selection from 'containers/Bucket/Selection'
@@ -15,9 +12,10 @@ import * as Add from './Add'
 import * as CreatePackage from './CreatePackage'
 import * as Get from './Get'
 import * as Organize from './Organize'
+import { useFeatures, type Features } from './useFeatures'
 
 export { DirHandleCreate as CreateHandle } from 'containers/Bucket/Toolbar'
-export { Add, CreatePackage, Get, Organize }
+export { Add, CreatePackage, Get, Organize, useFeatures, type Features }
 
 const useStyles = M.makeStyles((t) => ({
   root: {
@@ -27,34 +25,6 @@ const useStyles = M.makeStyles((t) => ({
   },
 }))
 
-interface Features {
-  add: boolean | null
-  get: false | { code: boolean } | null
-  organize: boolean | null
-  createPackage: boolean | null
-}
-
-export function useFeatures(): Features | null {
-  const { prefs } = BucketPreferences.use()
-  return React.useMemo(
-    () =>
-      BucketPreferences.Result.match(
-        {
-          Ok: ({ ui: { actions, blocks } }) => ({
-            add: actions.writeFile,
-            get:
-              !cfg.noDownload && actions.downloadObject ? { code: blocks.code } : false,
-            organize: true,
-            createPackage: actions.createPackage,
-          }),
-          _: () => null,
-        },
-        prefs,
-      ),
-    [prefs],
-  )
-}
-
 interface DirToolbarProps {
   className?: string
   features: Features | null
@@ -123,14 +93,14 @@ function DirToolbar({ className, features, handle, onReload }: DirToolbarProps)
 
         {features.get && (
           <Toolbar.Get>
-            <Get.Options handle={handle} hideCode={!features.get.code} />
+            <Get.Options handle={handle} features={features.get} />
           </Toolbar.Get>
         )}
 
         {features.organize && (
           <Organize.Context.Provider onReload={onReload}>
             <Toolbar.Organize>
-              <Organize.Options />
+              <Organize.Options features={features.organize} />
             </Toolbar.Organize>
           </Organize.Context.Provider>
         )}