better error handling and multi-delete support

wandb · Dec 19, 2024 · 0162f69 · 0162f69
1 parent 873dc27
commit 0162f69
Show file tree

Hide file tree

Showing 6 changed files with 178 additions and 23 deletions.
diff --git a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/ObjectVersionsPage.tsx b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/ObjectVersionsPage.tsx
@@ -15,6 +15,7 @@ import {
   GridRowSelectionModel,
   GridRowsProp,
 } from '@mui/x-data-grid-pro';
+import {useViewerInfo} from '@wandb/weave/common/hooks/useViewerInfo';
 import {Checkbox} from '@wandb/weave/components/Checkbox';
 import _ from 'lodash';
 import React, {useEffect, useMemo, useState} from 'react';
@@ -32,6 +33,10 @@ import {
 } from '../context';
 import {StyledDataGrid} from '../StyledDataGrid';
 import {basicField} from './common/DataTable';
+import {
+  DeleteObjectButtonWithModal,
+  DeleteObjectVersionsButtonWithModal,
+} from './common/DeleteModal';
 import {Empty} from './common/Empty';
 import {
   EMPTY_PROPS_ACTION_SPECS,
@@ -83,6 +88,7 @@ export const ObjectVersionsPage: React.FC<{
   onFilterUpdate?: (filter: WFHighLevelObjectVersionFilter) => void;
 }> = props => {
   const history = useHistory();
+  const {loading: loadingUserInfo, userInfo} = useViewerInfo();
   const router = useWeaveflowCurrentRouteContext();
   const [filter, setFilter] = useControllableState(
     props.initialFilter ?? {},
@@ -103,21 +109,29 @@ export const ObjectVersionsPage: React.FC<{
     return 'All Objects';
   }, [filter.objectName, filter.baseObjectClass]);
 
+  if (loadingUserInfo) {
+    return <Loading />;
+  }
+
   const hasComparison = filter.objectName != null;
-  const headerExtra = hasComparison ? (
-    <Button
-      className="mr-16"
-      disabled={selectedVersions.length < 2}
-      onClick={onCompare}>
-      Compare
-    </Button>
-  ) : undefined;
+  const viewer = userInfo ? userInfo.id : null;
+  const isReadonly = !viewer || !userInfo?.teams.includes(props.entity);
 
   return (
     <SimplePageLayout
       title={title}
       hideTabsIfSingle
-      headerExtra={headerExtra}
+      headerExtra={
+        <ObjectVersionsPageHeaderExtra
+          entity={entity}
+          project={project}
+          objectName={filter.objectName ?? null}
+          selectedVersions={selectedVersions}
+          showDeleteButton={!isReadonly}
+          showCompareButton={hasComparison}
+          onCompare={onCompare}
+        />
+      }
       tabs={[
         {
           label: '',
@@ -138,6 +152,52 @@ export const ObjectVersionsPage: React.FC<{
   );
 };
 
+const ObjectVersionsPageHeaderExtra: React.FC<{
+  entity: string;
+  project: string;
+  objectName: string | null;
+  selectedVersions: string[];
+  showDeleteButton?: boolean;
+  showCompareButton?: boolean;
+  onCompare: () => void;
+}> = ({
+  entity,
+  project,
+  objectName,
+  selectedVersions,
+  showDeleteButton,
+  showCompareButton,
+  onCompare,
+}) => {
+  const compareButton = showCompareButton ? (
+    <Button
+      className="mr-16"
+      disabled={selectedVersions.length < 2}
+      onClick={onCompare}>
+      Compare
+    </Button>
+  ) : undefined;
+
+  const deleteButton = showDeleteButton ? (
+    <div className="">
+      <DeleteObjectVersionsButtonWithModal
+        entity={entity}
+        project={project}
+        objectName={objectName ?? ''}
+        objectDigests={selectedVersions}
+        disabled={selectedVersions.length === 0 || !objectName}
+      />
+    </div>
+  ) : undefined;
+
+  return (
+    <div className="items-center gap-4">
+      {compareButton}
+      {deleteButton}
+    </div>
+  );
+};
+
 export type WFHighLevelObjectVersionFilter = {
   objectName?: string | null;
   baseObjectClass?: KnownBaseObjectClassType | null;

diff --git a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/common/DeleteModal.tsx b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/common/DeleteModal.tsx
@@ -6,6 +6,10 @@ import {
 } from '@material-ui/core';
 import {Button} from '@wandb/weave/components/Button';
 import {Tailwind} from '@wandb/weave/components/Tailwind';
+import {
+  maybePluralize,
+  maybePluralizeWord,
+} from '@wandb/weave/core/util/string';
 import React, {useState} from 'react';
 import styled from 'styled-components';
 
@@ -20,7 +24,8 @@ interface DeleteModalProps {
   open: boolean;
   onClose: () => void;
   onDelete: () => Promise<void>;
-  deleteTargetStr: string;
+  deleteTitleStr: string;
+  deleteBodyStrs?: string[];
   onSuccess?: () => void;
 }
 
@@ -36,7 +41,8 @@ export const DeleteModal: React.FC<DeleteModalProps> = ({
   open,
   onClose,
   onDelete,
-  deleteTargetStr,
+  deleteTitleStr,
+  deleteBodyStrs,
   onSuccess,
 }) => {
   const [deleteLoading, setDeleteLoading] = useState(false);
@@ -57,6 +63,8 @@ export const DeleteModal: React.FC<DeleteModalProps> = ({
       });
   };
 
+  const deleteBodyStrRes = deleteBodyStrs ? deleteBodyStrs : [deleteTitleStr];
+
   return (
     <Dialog
       open={open}
@@ -65,7 +73,7 @@ export const DeleteModal: React.FC<DeleteModalProps> = ({
         setError(null);
       }}>
       <Tailwind>
-        <DialogTitle>Delete {deleteTargetStr}</DialogTitle>
+        <DialogTitle>Delete {deleteTitleStr}</DialogTitle>
         <DialogContent className="overflow-hidden">
           <div className="mb-16">
             {error != null ? (
@@ -74,14 +82,20 @@ export const DeleteModal: React.FC<DeleteModalProps> = ({
               <p>Are you sure you want to delete?</p>
             )}
           </div>
-          <span className="text-md mt-10 font-semibold">{deleteTargetStr}</span>
+          <span className="text-md mt-10 font-semibold">
+            {deleteBodyStrRes.map((str, i) => (
+              <div key={i}>
+                <span>{str}</span>
+              </div>
+            ))}
+          </span>
         </DialogContent>
         <DialogActions $align="left">
           <Button
             variant="destructive"
             disabled={error != null || deleteLoading}
             onClick={handleDelete}>
-            {`Delete ${deleteTargetStr}`}
+            {`Delete ${deleteTitleStr}`}
           </Button>
           <Button
             variant="ghost"
@@ -143,7 +157,7 @@ export const DeleteObjectButtonWithModal: React.FC<{
       <DeleteModal
         open={deleteModalOpen}
         onClose={() => setDeleteModalOpen(false)}
-        deleteTargetStr={deleteStr}
+        deleteTitleStr={deleteStr}
         onDelete={() => objectVersionDelete(objVersionSchema)}
         onSuccess={closePeek}
       />
@@ -173,14 +187,49 @@ export const DeleteOpButtonWithModal: React.FC<{
       <DeleteModal
         open={deleteModalOpen}
         onClose={() => setDeleteModalOpen(false)}
-        deleteTargetStr={deleteStr}
+        deleteTitleStr={deleteStr}
         onDelete={() => opVersionDelete(opVersionSchema)}
         onSuccess={closePeek}
       />
     </>
   );
 };
 
+export const DeleteObjectVersionsButtonWithModal: React.FC<{
+  entity: string;
+  project: string;
+  objectName: string;
+  objectDigests: string[];
+  disabled?: boolean;
+}> = ({entity, project, objectName, objectDigests, disabled}) => {
+  const {useObjectDeleteFunc} = useWFHooks();
+  const {objectVersionsDelete} = useObjectDeleteFunc();
+  const [deleteModalOpen, setDeleteModalOpen] = useState(false);
+
+  const numObjects = objectDigests.length;
+  const versionsStr = maybePluralizeWord(numObjects, 'version', 's');
+
+  return (
+    <>
+      <Button
+        icon="delete"
+        variant="ghost"
+        onClick={() => setDeleteModalOpen(true)}
+        disabled={disabled}
+      />
+      <DeleteModal
+        open={deleteModalOpen}
+        onClose={() => setDeleteModalOpen(false)}
+        deleteTitleStr={`${numObjects} ${objectName} ${versionsStr}`}
+        deleteBodyStrs={objectDigests}
+        onDelete={() =>
+          objectVersionsDelete(entity, project, objectName, objectDigests)
+        }
+      />
+    </>
+  );
+};
+
 function makeDefaultObjectDeleteStr(objVersionSchema: ObjectVersionSchema) {
   return `${objVersionSchema.objectId}:v${objVersionSchema.versionIndex}`;
 }

diff --git a/...c/components/PagePanelComponents/Home/Browse3/pages/wfReactInterface/traceServerClient.ts b/...c/components/PagePanelComponents/Home/Browse3/pages/wfReactInterface/traceServerClient.ts
@@ -137,9 +137,15 @@ export class TraceServerClient extends CachingTraceServerClient {
     return res;
   }
 
-  public objectDelete(req: TraceObjDeleteReq): Promise<void> {
-    const res = super.objectDelete(req).then(() => {
+  public objectDelete(
+    req: TraceObjDeleteReq
+  ): Promise<void | {detail: string}> {
+    const res = super.objectDelete(req).then(res => {
+      if (res && 'detail' in res) {
+        throw new Error(res.detail);
+      }
       this.onObjectListeners.forEach(listener => listener());
+      return res;
     });
     return res;
   }

diff --git a/...onents/PagePanelComponents/Home/Browse3/pages/wfReactInterface/traceServerDirectClient.ts b/...onents/PagePanelComponents/Home/Browse3/pages/wfReactInterface/traceServerDirectClient.ts
@@ -231,7 +231,9 @@ export class DirectTraceServerClient {
     return this.makeRequest<TraceObjReadReq, TraceObjReadRes>('/obj/read', req);
   }
 
-  public objectDelete(req: TraceObjDeleteReq): Promise<void> {
+  public objectDelete(
+    req: TraceObjDeleteReq
+  ): Promise<void | {detail: string}> {
     return this.makeRequest<TraceObjDeleteReq, void>('/obj/delete', req);
   }
 

diff --git a/...rc/components/PagePanelComponents/Home/Browse3/pages/wfReactInterface/tsDataModelHooks.ts b/...rc/components/PagePanelComponents/Home/Browse3/pages/wfReactInterface/tsDataModelHooks.ts
@@ -1150,6 +1150,31 @@ const useObjectDeleteFunc = () => {
     [getTsClient, updateObjectCaches]
   );
 
+  const objectVersionsDelete = useCallback(
+    (entity: string, project: string, objectId: string, digests: string[]) => {
+      digests.forEach(digest => {
+        updateObjectCaches({
+          scheme: 'weave',
+          weaveKind: 'object',
+          entity,
+          project,
+          objectId,
+          versionHash: digest,
+          path: '',
+        });
+      });
+      return getTsClient().objectDelete({
+        project_id: projectIdFromParts({
+          entity: entity,
+          project: project,
+        }),
+        object_id: objectId,
+        digests,
+      });
+    },
+    [getTsClient, updateObjectCaches]
+  );
+
   const objectDeleteAllVersions = useCallback(
     (key: ObjectVersionKey) => {
       updateObjectCaches(key);
@@ -1198,6 +1223,7 @@ const useObjectDeleteFunc = () => {
   return {
     objectVersionDelete,
     objectDeleteAllVersions,
+    objectVersionsDelete,
     opVersionDelete,
     opDeleteAllVersions,
   };

diff --git a/...ents/PagePanelComponents/Home/Browse3/pages/wfReactInterface/wfDataModelHooksInterface.ts b/...ents/PagePanelComponents/Home/Browse3/pages/wfReactInterface/wfDataModelHooksInterface.ts
@@ -253,10 +253,22 @@ export type WFDataModelHooksInterface = {
     opts?: {skip?: boolean; noAutoRefresh?: boolean}
   ) => LoadableWithError<ObjectVersionSchema[]>;
   useObjectDeleteFunc: () => {
-    objectVersionDelete: (key: ObjectVersionKey) => Promise<void>;
-    objectDeleteAllVersions: (key: ObjectVersionKey) => Promise<void>;
-    opVersionDelete: (key: OpVersionKey) => Promise<void>;
-    opDeleteAllVersions: (key: OpVersionKey) => Promise<void>;
+    objectVersionDelete: (
+      key: ObjectVersionKey
+    ) => Promise<void | {detail: string}>;
+    objectDeleteAllVersions: (
+      key: ObjectVersionKey
+    ) => Promise<void | {detail: string}>;
+    objectVersionsDelete: (
+      entity: string,
+      project: string,
+      objectId: string,
+      digests: string[]
+    ) => Promise<void | {detail: string}>;
+    opVersionDelete: (key: OpVersionKey) => Promise<void | {detail: string}>;
+    opDeleteAllVersions: (
+      key: OpVersionKey
+    ) => Promise<void | {detail: string}>;
   };
   // `useRefsData` is in beta while we integrate Shawn's new Object DB
   useRefsData: (refUris: string[], tableQuery?: TableQuery) => Loadable<any[]>;