Reducing the amount of context needed to change card properties (#2579)

Co-authored-by: Mattermod <mattermod@users.noreply.github.com>

webapp/src/blocks/board.ts

@@ -186,12 +186,44 @@ function isPropertyEqual(propA: IPropertyTemplate, propB: IPropertyTemplate): bo
     return true
 }
 
+// createCardPropertiesPatches creates two BoardPatch instances, one that
+// contains the delta to update the board cardProperties and another one for
+// the undo action, in case it happens
+function createCardPropertiesPatches(newCardProperties: IPropertyTemplate[], oldCardProperties: IPropertyTemplate[]): BoardPatch[] {
+    const newDeletedCardProperties = getPropertiesDifference(newCardProperties, oldCardProperties)
+    const oldDeletedCardProperties = getPropertiesDifference(oldCardProperties, newCardProperties)
+    const newUpdatedCardProperties: IPropertyTemplate[] = []
+    newCardProperties.forEach((val) => {
+        const oldCardProperty = oldCardProperties.find((o) => o.id === val.id)
+        if (!oldCardProperty || !isPropertyEqual(val, oldCardProperty)) {
+            newUpdatedCardProperties.push(val)
+        }
+    })
+    const oldUpdatedCardProperties: IPropertyTemplate[] = []
+    oldCardProperties.forEach((val) => {
+        const newCardProperty = newCardProperties.find((o) => o.id === val.id)
+        if (!newCardProperty || !isPropertyEqual(val, newCardProperty)) {
+            oldUpdatedCardProperties.push(val)
+        }
+    })
+
+    return [
+        {
+            updatedCardProperties: newUpdatedCardProperties,
+            deletedCardProperties: oldDeletedCardProperties,
+        },
+        {
+            updatedCardProperties: oldUpdatedCardProperties,
+            deletedCardProperties: newDeletedCardProperties,
+        },
+    ]
+}
+
 // createPatchesFromBoards creates two BoardPatch instances, one that
 // contains the delta to update the board and another one for the undo
 // action, in case it happens
 function createPatchesFromBoards(newBoard: Board, oldBoard: Board): BoardPatch[] {
     const newDeletedProperties = difference(Object.keys(newBoard.properties || {}), Object.keys(oldBoard.properties || {}))
-    const newDeletedCardProperties = getPropertiesDifference(newBoard.cardProperties, oldBoard.cardProperties)
     const newDeletedColumnCalculations = difference(Object.keys(newBoard.columnCalculations), Object.keys(oldBoard.columnCalculations))
 
     const newUpdatedProperties: Record<string, any> = {}
@@ -200,13 +232,6 @@ function createPatchesFromBoards(newBoard: Board, oldBoard: Board): BoardPatch[]
             newUpdatedProperties[val] = newBoard.properties[val]
         }
     })
-    const newUpdatedCardProperties: IPropertyTemplate[] = []
-    newBoard.cardProperties.forEach((val) => {
-        const oldCardProperty = oldBoard.cardProperties.find((o) => o.id === val.id)
-        if (!oldCardProperty || !isPropertyEqual(val, oldCardProperty)) {
-            newUpdatedCardProperties.push(val)
-        }
-    })
     const newUpdatedColumnCalculations: Record<string, any> = {}
     Object.keys(newBoard.columnCalculations).forEach((val) => {
         if (oldBoard.columnCalculations[val] !== newBoard.columnCalculations[val]) {
@@ -225,7 +250,6 @@ function createPatchesFromBoards(newBoard: Board, oldBoard: Board): BoardPatch[]
     })
 
     const oldDeletedProperties = difference(Object.keys(oldBoard.properties || {}), Object.keys(newBoard.properties || {}))
-    const oldDeletedCardProperties = getPropertiesDifference(oldBoard.cardProperties, newBoard.cardProperties)
     const oldDeletedColumnCalculations = difference(Object.keys(oldBoard.columnCalculations), Object.keys(newBoard.columnCalculations))
 
     const oldUpdatedProperties: Record<string, any> = {}
@@ -234,13 +258,6 @@ function createPatchesFromBoards(newBoard: Board, oldBoard: Board): BoardPatch[]
             oldUpdatedProperties[val] = oldBoard.properties[val]
         }
     })
-    const oldUpdatedCardProperties: IPropertyTemplate[] = []
-    oldBoard.cardProperties.forEach((val) => {
-        const newCardProperty = newBoard.cardProperties.find((o) => o.id === val.id)
-        if (!newCardProperty || !isPropertyEqual(val, newCardProperty)) {
-            oldUpdatedCardProperties.push(val)
-        }
-    })
     const oldUpdatedColumnCalculations: Record<string, any> = {}
     Object.keys(oldBoard.columnCalculations).forEach((val) => {
         if (newBoard.columnCalculations[val] !== oldBoard.columnCalculations[val]) {
@@ -258,22 +275,22 @@ function createPatchesFromBoards(newBoard: Board, oldBoard: Board): BoardPatch[]
         }
     })
 
+    const [cardPropertiesPatch, cardPropertiesUndoPatch] = createCardPropertiesPatches(newBoard.cardProperties, oldBoard.cardProperties)
+
     return [
         {
             ...newData,
+            ...cardPropertiesPatch,
             updatedProperties: newUpdatedProperties,
             deletedProperties: oldDeletedProperties,
-            updatedCardProperties: newUpdatedCardProperties,
-            deletedCardProperties: oldDeletedCardProperties,
             updatedColumnCalculations: newUpdatedColumnCalculations,
             deletedColumnCalculations: oldDeletedColumnCalculations,
         },
         {
             ...oldData,
+            ...cardPropertiesUndoPatch,
             updatedProperties: oldUpdatedProperties,
             deletedProperties: newDeletedProperties,
-            updatedCardProperties: oldUpdatedCardProperties,
-            deletedCardProperties: newDeletedCardProperties,
             updatedColumnCalculations: oldUpdatedColumnCalculations,
             deletedColumnCalculations: newDeletedColumnCalculations,
         },
@@ -324,4 +341,5 @@ export {
     BoardTypePrivate,
     createPatchesFromBoards,
     createPatchesFromBoardsAndBlocks,
+    createCardPropertiesPatches,
 }

webapp/src/components/kanban/kanban.test.tsx

@@ -450,7 +450,7 @@ describe('src/component/kanban/kanban', () => {
         fireEvent.blur(inputTitle)
 
         await waitFor(async () => {
-            expect(mockedchangePropertyOptionValue).toBeCalledWith(board, groupProperty, optionQ1, 'New Q1')
+            expect(mockedchangePropertyOptionValue).toBeCalledWith(board.id, board.cardProperties, groupProperty, optionQ1, 'New Q1')
         })
 
         expect(container).toMatchSnapshot()

webapp/src/components/kanban/kanban.tsx

@@ -66,7 +66,7 @@ const Kanban = (props: Props) => {
     const visibleBadges = activeView.fields.visiblePropertyIds.includes(Constants.badgesColumnId)
 
     const propertyNameChanged = useCallback(async (option: IPropertyOption, text: string): Promise<void> => {
-        await mutator.changePropertyOptionValue(board, groupByProperty!, option, text)
+        await mutator.changePropertyOptionValue(board.id, board.cardProperties, groupByProperty!, option, text)
     }, [board, groupByProperty])
 
     const addGroupClicked = useCallback(async () => {
@@ -78,7 +78,7 @@ const Kanban = (props: Props) => {
             color: 'propColorDefault',
         }
 
-        await mutator.insertPropertyOption(board, groupByProperty!, option, 'add group')
+        await mutator.insertPropertyOption(board.id, board.cardProperties, groupByProperty!, option, 'add group')
     }, [board, groupByProperty])
 
     const orderAfterMoveToColumn = useCallback((cardIds: string[], columnId?: string): string[] => {

webapp/src/components/kanban/kanbanColumnHeader.tsx

@@ -172,7 +172,7 @@ export default function KanbanColumnHeader(props: Props): JSX.Element {
                                             id='delete'
                                             icon={<DeleteIcon/>}
                                             name={intl.formatMessage({id: 'BoardComponent.delete', defaultMessage: 'Delete'})}
-                                            onClick={() => mutator.deletePropertyOption(board, groupByProperty!, group.option)}
+                                            onClick={() => mutator.deletePropertyOption(board.id, board.cardProperties, groupByProperty!, group.option)}
                                         />
                                         <Menu.Separator/>
                                         {Object.entries(Constants.menuColors).map(([key, color]) => (
@@ -180,7 +180,7 @@ export default function KanbanColumnHeader(props: Props): JSX.Element {
                                                 key={key}
                                                 id={key}
                                                 name={color}
-                                                onClick={() => mutator.changePropertyOptionColor(board, groupByProperty!, group.option, key)}
+                                                onClick={() => mutator.changePropertyOptionColor(board.id, board.cardProperties, groupByProperty!, group.option, key)}
                                             />
                                         ))}
                                     </>}

webapp/src/components/propertyValueElement.tsx

@@ -82,7 +82,7 @@ const PropertyValueElement = (props:Props): JSX.Element => {
             color: 'propColorDefault',
         }
         currentValues.push(option)
-        mutator.insertPropertyOption(board, propertyTemplate, option, 'add property option').then(() => {
+        mutator.insertPropertyOption(board.id, board.cardProperties, propertyTemplate, option, 'add property option').then(() => {
             mutator.changePropertyValue(props.board.id, card, propertyTemplate.id, currentValues.map((v: IPropertyOption) => v.id))
         })
     }, [board, props.board.id, card, propertyTemplate])
@@ -95,8 +95,8 @@ const PropertyValueElement = (props:Props): JSX.Element => {
         }
     }, [value, props.board.id, card, propertyTemplate.id])
     const onChangeInMultiselect = useCallback((newValue) => mutator.changePropertyValue(props.board.id, card, propertyTemplate.id, newValue), [props.board.id, card, propertyTemplate])
-    const onChangeColorInMultiselect = useCallback((option: IPropertyOption, colorId: string) => mutator.changePropertyOptionColor(board, propertyTemplate, option, colorId), [board, propertyTemplate])
+    const onChangeColorInMultiselect = useCallback((option: IPropertyOption, colorId: string) => mutator.changePropertyOptionColor(board.id, board.cardProperties, propertyTemplate, option, colorId), [board, propertyTemplate])
-    const onDeleteOptionInMultiselect = useCallback((option: IPropertyOption) => mutator.deletePropertyOption(board, propertyTemplate, option), [board, propertyTemplate])
+    const onDeleteOptionInMultiselect = useCallback((option: IPropertyOption) => mutator.deletePropertyOption(board.id, board.cardProperties, propertyTemplate, option), [board, propertyTemplate])
 
     const onCreateInSelect = useCallback((newValue) => {
         const option: IPropertyOption = {
@@ -104,14 +104,14 @@ const PropertyValueElement = (props:Props): JSX.Element => {
             value: newValue,
             color: 'propColorDefault',
         }
-        mutator.insertPropertyOption(board, propertyTemplate, option, 'add property option').then(() => {
+        mutator.insertPropertyOption(board.id, board.cardProperties, propertyTemplate, option, 'add property option').then(() => {
             mutator.changePropertyValue(props.board.id, card, propertyTemplate.id, option.id)
         })
     }, [board, props.board.id, card, propertyTemplate.id])
 
     const onChangeInSelect = useCallback((newValue) => mutator.changePropertyValue(props.board.id, card, propertyTemplate.id, newValue), [])
-    const onChangeColorInSelect = useCallback((option: IPropertyOption, colorId: string) => mutator.changePropertyOptionColor(board, propertyTemplate, option, colorId), [board, propertyTemplate])
+    const onChangeColorInSelect = useCallback((option: IPropertyOption, colorId: string) => mutator.changePropertyOptionColor(board.id, board.cardProperties, propertyTemplate, option, colorId), [board, propertyTemplate])
-    const onDeleteOptionInSelect = useCallback((option: IPropertyOption) => mutator.deletePropertyOption(board, propertyTemplate, option), [board, propertyTemplate])
+    const onDeleteOptionInSelect = useCallback((option: IPropertyOption) => mutator.deletePropertyOption(board.id, board.cardProperties, propertyTemplate, option), [board, propertyTemplate])
 
     const validateProp = useCallback((val: string): boolean => {
         if (val === '') {

webapp/src/components/table/table.tsx

@@ -180,7 +180,7 @@ const Table = (props: Props): JSX.Element => {
     }, [activeView, cards, props.selectedCardIds, groupByProperty])
 
     const propertyNameChanged = useCallback(async (option: IPropertyOption, text: string): Promise<void> => {
-        await mutator.changePropertyOptionValue(board, groupByProperty!, option, text)
+        await mutator.changePropertyOptionValue(board.id, board.cardProperties, groupByProperty!, option, text)
     }, [board, groupByProperty])
 
     return (

webapp/src/components/table/tableGroupHeaderRow.tsx

@@ -125,7 +125,7 @@ const TableGroupHeaderRow = (props: Props): JSX.Element => {
                                         id='delete'
                                         icon={<DeleteIcon/>}
                                         name={intl.formatMessage({id: 'BoardComponent.delete', defaultMessage: 'Delete'})}
-                                        onClick={() => mutator.deletePropertyOption(board, groupByProperty!, group.option)}
+                                        onClick={() => mutator.deletePropertyOption(board.id, board.cardProperties, groupByProperty!, group.option)}
                                     />
                                     <Menu.Separator/>
                                     {Object.entries(Constants.menuColors).map(([key, color]) => (
@@ -133,7 +133,7 @@ const TableGroupHeaderRow = (props: Props): JSX.Element => {
                                             key={key}
                                             id={key}
                                             name={color}
-                                            onClick={() => mutator.changePropertyOptionColor(board, groupByProperty!, group.option, key)}
+                                            onClick={() => mutator.changePropertyOptionColor(board.id, board.cardProperties, groupByProperty!, group.option, key)}
                                         />
                                     ))}
                                 </>}

webapp/src/mutator.ts

@@ -3,10 +3,11 @@
 
 import {IntlShape} from 'react-intl'
 import {batch} from 'react-redux'
+import cloneDeep from 'lodash/cloneDeep'
 
 import {BlockIcons} from './blockIcons'
 import {Block, BlockPatch, createPatchesFromBlocks} from './blocks/block'
-import {Board, BoardMember, BoardsAndBlocks, IPropertyOption, IPropertyTemplate, PropertyType, createBoard, createPatchesFromBoards, createPatchesFromBoardsAndBlocks} from './blocks/board'
+import {Board, BoardMember, BoardsAndBlocks, IPropertyOption, IPropertyTemplate, PropertyType, createBoard, createPatchesFromBoards, createPatchesFromBoardsAndBlocks, createCardPropertiesPatches} from './blocks/board'
 import {BoardView, ISortOption, createBoardView, KanbanCalculationFields} from './blocks/boardView'
 import {Card, createCard} from './blocks/card'
 import {ContentBlock} from './blocks/contentBlock'
@@ -559,52 +560,66 @@ class Mutator {
 
     // Properties
 
-    async insertPropertyOption(board: Board, template: IPropertyTemplate, option: IPropertyOption, description = 'add option') {
+    async updateBoardCardProperties(boardId: string, oldProperties: IPropertyTemplate[], newProperties: IPropertyTemplate[], description = 'update card properties') {
-        Utils.assert(board.cardProperties.includes(template))
+        const [updatePatch, undoPatch] = createCardPropertiesPatches(newProperties, oldProperties)
+        await undoManager.perform(
+            async () => {
+                await octoClient.patchBoard(boardId, updatePatch)
+            },
+            async () => {
+                await octoClient.patchBoard(boardId, undoPatch)
+            },
+            description,
+            this.undoGroupId,
+        )
+    }
 
-        const newBoard = createBoard(board)
+    async insertPropertyOption(boardId: string, oldCardProperties: IPropertyTemplate[], template: IPropertyTemplate, option: IPropertyOption, description = 'add option') {
-        const newTemplate = newBoard.cardProperties.find((o: IPropertyTemplate) => o.id === template.id)!
+        Utils.assert(oldCardProperties.includes(template))
+
+        const newCardProperties: IPropertyTemplate[] = cloneDeep(oldCardProperties)
+        const newTemplate = newCardProperties.find((o: IPropertyTemplate) => o.id === template.id)!
         newTemplate.options.push(option)
 
-        await this.updateBoard(newBoard, board, description)
+        await this.updateBoardCardProperties(boardId, oldCardProperties, newCardProperties, description)
     }
 
-    async deletePropertyOption(board: Board, template: IPropertyTemplate, option: IPropertyOption) {
+    async deletePropertyOption(boardId: string, oldCardProperties: IPropertyTemplate[], template: IPropertyTemplate, option: IPropertyOption) {
-        const newBoard = createBoard(board)
+        const newCardProperties: IPropertyTemplate[] = cloneDeep(oldCardProperties)
-        const newTemplate = newBoard.cardProperties.find((o: IPropertyTemplate) => o.id === template.id)!
+        const newTemplate = newCardProperties.find((o: IPropertyTemplate) => o.id === template.id)!
         newTemplate.options = newTemplate.options.filter((o) => o.id !== option.id)
 
-        await this.updateBoard(newBoard, board, 'delete option')
+        await this.updateBoardCardProperties(boardId, oldCardProperties, newCardProperties, 'delete option')
     }
 
-    async changePropertyOptionOrder(board: Board, template: IPropertyTemplate, option: IPropertyOption, destIndex: number) {
+    async changePropertyOptionOrder(boardId: string, oldCardProperties: IPropertyTemplate[], template: IPropertyTemplate, option: IPropertyOption, destIndex: number) {
         const srcIndex = template.options.indexOf(option)
         Utils.log(`srcIndex: ${srcIndex}, destIndex: ${destIndex}`)
 
-        const newBoard = createBoard(board)
+        const newCardProperties: IPropertyTemplate[] = cloneDeep(oldCardProperties)
-        const newTemplate = newBoard.cardProperties.find((o: IPropertyTemplate) => o.id === template.id)!
+        const newTemplate = newCardProperties.find((o: IPropertyTemplate) => o.id === template.id)!
         newTemplate.options.splice(destIndex, 0, newTemplate.options.splice(srcIndex, 1)[0])
 
-        await this.updateBoard(newBoard, board, 'reorder options')
+        await this.updateBoardCardProperties(boardId, oldCardProperties, newCardProperties, 'reorder option')
     }
 
-    async changePropertyOptionValue(board: Board, propertyTemplate: IPropertyTemplate, option: IPropertyOption, value: string) {
+    async changePropertyOptionValue(boardId: string, oldCardProperties: IPropertyTemplate[], propertyTemplate: IPropertyTemplate, option: IPropertyOption, value: string) {
-        const newBoard = createBoard(board)
+        const newCardProperties: IPropertyTemplate[] = cloneDeep(oldCardProperties)
-        const newTemplate = newBoard.cardProperties.find((o: IPropertyTemplate) => o.id === propertyTemplate.id)!
+        const newTemplate = newCardProperties.find((o: IPropertyTemplate) => o.id === propertyTemplate.id)!
         const newOption = newTemplate.options.find((o) => o.id === option.id)!
         newOption.value = value
 
-        await this.updateBoard(newBoard, board, 'rename option')
+        await this.updateBoardCardProperties(boardId, oldCardProperties, newCardProperties, 'rename option')
 
-        return newBoard
+        return newCardProperties
     }
 
-    async changePropertyOptionColor(board: Board, template: IPropertyTemplate, option: IPropertyOption, color: string) {
+    async changePropertyOptionColor(boardId: string, oldCardProperties: IPropertyTemplate[], template: IPropertyTemplate, option: IPropertyOption, color: string) {
-        const newBoard = createBoard(board)
+        const newCardProperties: IPropertyTemplate[] = cloneDeep(oldCardProperties)
-        const newTemplate = newBoard.cardProperties.find((o: IPropertyTemplate) => o.id === template.id)!
+        const newTemplate = newCardProperties.find((o: IPropertyTemplate) => o.id === template.id)!
         const newOption = newTemplate.options.find((o) => o.id === option.id)!
         newOption.color = color
-        await this.updateBoard(newBoard, board, 'change option color')
+        await this.updateBoardCardProperties(boardId, oldCardProperties, newCardProperties, 'rename option')
     }
 
     async changePropertyValue(boardId: string, card: Card, propertyId: string, value?: string | string[], description = 'change property') {