187178550 v3 DI Delete Improvements (#1397)

* Allow deleting multiple items in one API request.

* Broadcast notifications when deleting cases.

* Limit delete and insert cases options in index column menu.

* Include codapVersion in get interactiveFrame responses.

v3/cypress/e2e/plugin.spec.ts

@@ -255,6 +255,12 @@ context("codap plugins", () => {
     webView.confirmAPITesterResponseContains(/"operation":\s"updateCases/)
     webView.clearAPITesterResponses()
 
+    cy.log("Broadcast deleteCases notifications")
+    table.openIndexMenuForRow(2)
+    table.deleteCase()
+    webView.confirmAPITesterResponseContains(/"operation":\s"deleteCases/)
+    webView.clearAPITesterResponses()
+
     cy.log("Broadcast updateCollection notifications")
     table.renameCollection("c1", "Mammals")
     webView.confirmAPITesterResponseContains(/"operation":\s"updateCollection/)

v3/src/components/case-table/index-menu-list.tsx

@@ -4,6 +4,7 @@ import { useDataSetContext } from "../../hooks/use-data-set-context"
 import { removeCasesWithCustomUndoRedo } from "../../models/data/data-set-undo"
 import { t } from "../../utilities/translation/translate"
 import { InsertCasesModal } from "./insert-cases-modal"
+import { isItemEditable } from "../../utilities/plugin-utils"
 
 interface IProps {
   caseId: string
@@ -14,7 +15,11 @@ export const IndexMenuList = ({caseId, index}: IProps) => {
   const toast = useToast()
   const data = useDataSetContext()
   const { isOpen, onOpen, onClose } = useDisclosure()
-  const deleteCasesItemText = data?.selection.size === 1
+  const deletableSelectedItems = data?.selection
+    ? Array.from(data.selection).filter(itemId => isItemEditable(data, itemId))
+    : []
+  const disableEdits = deletableSelectedItems.length < 1
+  const deleteCasesItemText = deletableSelectedItems.length === 1
                                 ? t("DG.CaseTable.indexMenu.deleteCase")
                                 : t("DG.CaseTable.indexMenu.deleteCases")
 
@@ -38,7 +43,7 @@ export const IndexMenuList = ({caseId, index}: IProps) => {
 
   const handleDeleteCases = () => {
     if (data?.selection.size) {
-      removeCasesWithCustomUndoRedo(data, Array.from(data.selection))
+      removeCasesWithCustomUndoRedo(data, deletableSelectedItems)
     }
   }
 
@@ -48,9 +53,15 @@ export const IndexMenuList = ({caseId, index}: IProps) => {
         <MenuItem onClick={()=>handleMenuItemClick("Move Data Entry Row")}>
           {t("DG.CaseTable.indexMenu.moveEntryRow")}
         </MenuItem>
-        <MenuItem onClick={handleInsertCase}>{t("DG.CaseTable.indexMenu.insertCase")}</MenuItem>
-        <MenuItem onClick={handleInsertCases}>{t("DG.CaseTable.indexMenu.insertCases")}</MenuItem>
-        <MenuItem onClick={handleDeleteCases}>{deleteCasesItemText}</MenuItem>
+        <MenuItem isDisabled={disableEdits} onClick={handleInsertCase}>
+          {t("DG.CaseTable.indexMenu.insertCase")}
+        </MenuItem>
+        <MenuItem isDisabled={disableEdits} onClick={handleInsertCases}>
+          {t("DG.CaseTable.indexMenu.insertCases")}
+        </MenuItem>
+        <MenuItem isDisabled={disableEdits} onClick={handleDeleteCases}>
+          {deleteCasesItemText}
+        </MenuItem>
       </MenuList>
       <InsertCasesModal caseId={caseId} isOpen={isOpen} onClose={onClose}/>
     </>

v3/src/data-interactive/data-interactive-types.ts

@@ -80,6 +80,7 @@ export interface DIGetCaseResult {
 export interface DIInteractiveFrame {
   allowEmptyAttributeDeletion?: boolean
   cannotClose?: boolean
+  codapVersion?: string
   dimensions?: {
     height?: number
     width?: number

v3/src/data-interactive/handlers/handler-functions.ts

@@ -23,16 +23,17 @@ export function deleteCaseBy(resources: DIResources, aCase?: ICase) {
   return { success: true as const, values: [toV2Id(aCase.__id__)] }
 }
 
-export function deleteItem(resources: DIResources, item?: ICase) {
+export function deleteItem(resources: DIResources, item?: ICase | string[]) {
   const { dataContext } = resources
   if (!dataContext) return dataContextNotFoundResult
   if (!item) return itemNotFoundResult
 
+  const itemIds = Array.isArray(item) ? item : [item.__id__]
   dataContext.applyModelChange(() => {
-    dataContext.removeCases([item.__id__])
+    dataContext.removeCases(itemIds)
   })
 
-  return { success: true as const, values: [toV2Id(item.__id__)] }
+  return { success: true as const, values: itemIds.map(itemId => toV2Id(itemId)) }
 }
 
 export function getCaseBy(resources: DIResources, aCase?: ICase) {

v3/src/data-interactive/handlers/interactive-frame-handler.ts

@@ -20,6 +20,7 @@ export const diInteractiveFrameHandler: DIHandler = {
     } = webViewContent ?? {}
     const values: DIInteractiveFrame = {
       allowEmptyAttributeDeletion,
+      codapVersion: appState.getVersion(),
       dimensions,
       externalUndoAvailable: true, // TODO Fix hard coded value
       id: toV2Id(interactiveFrame.id),

v3/src/data-interactive/handlers/item-handler.test.ts

@@ -55,6 +55,13 @@ describe("DataInteractive ItemHandler", () => {
     const values = result.values as number[]
     expect(values[0]).toBe(toV2Id(itemId))
     expect(dataContext.getItem(itemId)).toBeUndefined()
+
+    const item2Id = dataContext.getItemAtIndex(2)!.__id__
+    const item3Id = dataContext.getItemAtIndex(3)!.__id__
+    const resultMultiple = handler.delete!({ dataContext }, [{ id: toV2Id(item2Id) }, { id: toV2Id(item3Id) }])
+    expect(resultMultiple?.success).toBe(true)
+    expect(dataContext.getItem(item2Id)).toBeUndefined()
+    expect(dataContext.getItem(item3Id)).toBeUndefined()
   })
 
   it("get works", () => {

v3/src/data-interactive/handlers/item-handler.ts

@@ -1,7 +1,7 @@
 import { createCasesNotification } from "../../models/data/data-set-notifications"
 import { toV2Id, toV3ItemId } from "../../utilities/codap-utils"
 import { registerDIHandler } from "../data-interactive-handler"
-import { DIHandler, DIItem, DIItemValues, DIResources, DIValues } from "../data-interactive-types"
+import { DIHandler, DIItem, DIItemValues, DINewCase, DIResources, DIValues } from "../data-interactive-types"
 import { deleteItem, getItem, updateCaseBy, updateCasesBy } from "./handler-functions"
 import { dataContextNotFoundResult, valuesRequiredResult } from "./di-results"
 
@@ -76,10 +76,16 @@ export const diItemHandler: DIHandler = {
     }
   },
 
-  delete(resources: DIResources) {
+  delete(resources: DIResources, values?: DIValues) {
     const { item } = resources
 
-    return deleteItem(resources, item)
+    let itemIds: string[] | undefined
+    if (!item && values && Array.isArray(values)) {
+      itemIds = (values as DINewCase[]).map(aCase => aCase.id != null && toV3ItemId(aCase.id))
+        .filter(id => !!id) as string[]
+    }
+
+    return deleteItem(resources, item ?? itemIds)
   },
 
   get(resources: DIResources) {

v3/src/lib/handle-cfm-event.ts

@@ -21,6 +21,7 @@ export function handleCFMEvent(cfmClient: CloudFileManagerClient, event: CloudFi
         appBuildNum: build.buildNumber
       })
       wrapCfmCallback(() => cfmClient._ui.setMenuBarInfo(`v${pkg.version} (${build.buildNumber})`))
+      appState.setVersion(pkg.version)
       break
     // case "requiresUserInteraction":
     //   break

v3/src/models/app-state.ts

@@ -30,6 +30,8 @@ class AppState {
   @observable
   private isPerformanceEnabled = true
 
+  private version = ""
+
   constructor() {
     this.currentDocument = createCodapDocument()
 
@@ -101,6 +103,14 @@ class AppState {
     this.isPerformanceEnabled = false
   }
 
+  setVersion(version: string) {
+    this.version = version
+  }
+
+  getVersion() {
+    return this.version
+  }
+
   @computed
   get appMode(): AppMode {
     return this.isPerformanceEnabled && (this.appModeCount > 0) ? "performance" : "normal"

v3/src/models/data/data-set-notifications.ts

@@ -123,6 +123,14 @@ export function updateCasesNotification(data: IDataSet, cases?: ICase[]) {
   return notification("updateCases", result, data)
 }
 
+export function deleteCasesNotification(data: IDataSet, cases?: ICase[]) {
+  const result = {
+    success: true,
+    cases: cases?.map(c => convertCaseToV2FullCase(c, data))
+  }
+  return notification("deleteCases", result, data)
+}
+
 // selectCasesNotification returns a function that will later be called to determine if the selection
 // actually changed and a notification is necessary to broadcast
 export function selectCasesNotification(dataset: IDataSet, extend?: boolean) {

v3/src/models/data/data-set-undo.ts

@@ -1,10 +1,11 @@
 import { IAnyStateTreeNode, resolveIdentifier } from "mobx-state-tree"
+import { ICustomUndoRedoPatcher } from "../history/custom-undo-redo-registry"
 import { HistoryEntryType } from "../history/history"
 import { ICustomPatch } from "../history/tree-types"
-import { ICustomUndoRedoPatcher } from "../history/custom-undo-redo-registry"
+import { withCustomUndoRedo } from "../history/with-custom-undo-redo"
 import { ICase, IItem } from "./data-set-types"
 import { DataSet, IDataSet } from "./data-set"
-import { withCustomUndoRedo } from "../history/with-custom-undo-redo"
+import { deleteCasesNotification, } from "./data-set-notifications"
 
 export interface ISetCaseValuesCustomPatch extends ICustomPatch {
   type: "DataSet.setCaseValues"
@@ -87,15 +88,19 @@ const removeCasesCustomUndoRedo: ICustomUndoRedoPatcher = {
 export function removeCasesWithCustomUndoRedo(data: IDataSet, caseIds: string[]) {
   data.validateCases()
 
-  // identify the items to remove
+  // identify the items to remove and build up removed cases for notification
   const itemIdsToRemove = new Set<string>()
+  const cases: ICase[] = []
   caseIds.forEach(caseId => {
     const caseGroup = data.caseInfoMap.get(caseId)
     if (caseGroup) {
       caseGroup.childItemIds.forEach(itemId => itemIdsToRemove.add(itemId))
+      cases.push(caseGroup.groupedCase)
     }
     else if (data.itemInfoMap.get(caseId) != null) {
       itemIdsToRemove.add(caseId)
+      const childCase = data.itemIdChildCaseMap.get(caseId)
+      if (childCase) cases.push(childCase.groupedCase)
     }
   })
 
@@ -155,6 +160,7 @@ export function removeCasesWithCustomUndoRedo(data: IDataSet, caseIds: string[])
 
     data.removeCases(caseIds)
   }, {
+    notify: deleteCasesNotification(data, cases),
     undoStringKey: "DG.Undo.data.deleteCases",
     redoStringKey: "DG.Redo.data.deleteCases"
   })

v3/src/models/data/data-set.ts

@@ -149,6 +149,8 @@ export const DataSet = V2Model.named("DataSet").props({
   selectionChanges: 0,
   // map from case ID to the CaseGroup it represents
   caseInfoMap: new Map<string, CaseInfo>(),
+  // map from item ID to the child case containing it
+  itemIdChildCaseMap: new Map<string, CaseInfo>(),
   transactionCount: 0,
   // the id of the interactive frame handling this dataset
   // used by the Collaborative plugin
@@ -475,6 +477,10 @@ export const DataSet = V2Model.named("DataSet").props({
         // update the caseGroupMap
         collection.caseGroups.forEach(group => self.caseInfoMap.set(group.groupedCase.__id__, group))
       })
+      self.itemIdChildCaseMap.clear()
+      self.childCollection.caseGroups.forEach(caseGroup => {
+        self.itemIdChildCaseMap.set(caseGroup.childItemIds[0], caseGroup)
+      })
       self.setValidCases()
     }
   }