Gh 1841 batch patches (#1935)

* Updating table row css (#1787)

* package patchBlocks as batches, move updateBlocks to transactional

* fix lint errors

* fix from review

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

server/api/api.go

@@ -69,6 +69,7 @@ func (a *API) RegisterRoutes(r *mux.Router) {
 
 	apiv1.HandleFunc("/workspaces/{workspaceID}/blocks", a.sessionRequired(a.handleGetBlocks)).Methods("GET")
 	apiv1.HandleFunc("/workspaces/{workspaceID}/blocks", a.sessionRequired(a.handlePostBlocks)).Methods("POST")
+	apiv1.HandleFunc("/workspaces/{workspaceID}/blocks", a.sessionRequired(a.handlePatchBlocks)).Methods("PATCH")
 	apiv1.HandleFunc("/workspaces/{workspaceID}/blocks/{blockID}", a.sessionRequired(a.handleDeleteBlock)).Methods("DELETE")
 	apiv1.HandleFunc("/workspaces/{workspaceID}/blocks/{blockID}", a.sessionRequired(a.handlePatchBlock)).Methods("PATCH")
 	apiv1.HandleFunc("/workspaces/{workspaceID}/blocks/{blockID}/subtree", a.attachSession(a.handleGetSubTree, false)).Methods("GET")
@@ -676,6 +677,77 @@ func (a *API) handlePatchBlock(w http.ResponseWriter, r *http.Request) {
 	auditRec.Success()
 }
 
+func (a *API) handlePatchBlocks(w http.ResponseWriter, r *http.Request) {
+	// swagger:operation PATCH /api/v1/workspaces/{workspaceID}/blocks/ patchBlocks
+	//
+	// Partially updates batch of blocks
+	//
+	// ---
+	// produces:
+	// - application/json
+	// parameters:
+	// - name: workspaceID
+	//   in: path
+	//   description: Workspace ID
+	//   required: true
+	//   type: string
+	// - name: Body
+	//   in: body
+	//   description: block Ids and block patches to apply
+	//   required: true
+	//   schema:
+	//     "$ref": "#/definitions/BlockPatchBatch"
+	// security:
+	// - BearerAuth: []
+	// responses:
+	//   '200':
+	//     description: success
+	//   default:
+	//     description: internal error
+	//     schema:
+	//       "$ref": "#/definitions/ErrorResponse"
+
+	ctx := r.Context()
+	session := ctx.Value(sessionContextKey).(*model.Session)
+	userID := session.UserID
+
+	container, err := a.getContainer(r)
+	if err != nil {
+		a.noContainerErrorResponse(w, r.URL.Path, err)
+		return
+	}
+
+	requestBody, err := ioutil.ReadAll(r.Body)
+	if err != nil {
+		a.errorResponse(w, r.URL.Path, http.StatusInternalServerError, "", err)
+		return
+	}
+
+	var patches *model.BlockPatchBatch
+	err = json.Unmarshal(requestBody, &patches)
+	if err != nil {
+		a.errorResponse(w, r.URL.Path, http.StatusInternalServerError, "", err)
+		return
+	}
+
+	auditRec := a.makeAuditRecord(r, "patchBlocks", audit.Fail)
+	defer a.audit.LogRecord(audit.LevelModify, auditRec)
+	for i := range patches.BlockIDs {
+		auditRec.AddMeta("block_"+strconv.FormatInt(int64(i), 10), patches.BlockIDs[i])
+	}
+
+	err = a.app.PatchBlocks(*container, patches, userID)
+	if err != nil {
+		a.errorResponse(w, r.URL.Path, http.StatusInternalServerError, "", err)
+		return
+	}
+
+	a.logger.Debug("PATCH Blocks", mlog.String("patches", strconv.Itoa(len(patches.BlockIDs))))
+	jsonStringResponse(w, http.StatusOK, "{}")
+
+	auditRec.Success()
+}
+
 func (a *API) handleGetSubTree(w http.ResponseWriter, r *http.Request) {
 	// swagger:operation GET /api/v1/workspaces/{workspaceID}/blocks/{blockID}/subtree getSubTree
 	//

server/app/blocks.go

@@ -62,6 +62,37 @@ func (a *App) PatchBlock(c store.Container, blockID string, blockPatch *model.Bl
 	return nil
 }
 
+func (a *App) PatchBlocks(c store.Container, blockPatches *model.BlockPatchBatch, userID string) error {
+	oldBlocks := make([]model.Block, 0, len(blockPatches.BlockIDs))
+	for _, blockID := range blockPatches.BlockIDs {
+		oldBlock, err := a.store.GetBlock(c, blockID)
+		if err != nil {
+			return nil
+		}
+		oldBlocks = append(oldBlocks, *oldBlock)
+	}
+
+	err := a.store.PatchBlocks(c, blockPatches, userID)
+	if err != nil {
+		return err
+	}
+
+	a.metrics.IncrementBlocksPatched(len(oldBlocks))
+	for i, blockID := range blockPatches.BlockIDs {
+		newBlock, err := a.store.GetBlock(c, blockID)
+		if err != nil {
+			return nil
+		}
+		a.wsAdapter.BroadcastBlockChange(c.WorkspaceID, *newBlock)
+		go func(currentIndex int) {
+			a.webhook.NotifyUpdate(*newBlock)
+			a.notifyBlockChanged(notify.Update, c, newBlock, &oldBlocks[currentIndex], userID)
+		}(i)
+	}
+
+	return nil
+}
+
 func (a *App) InsertBlock(c store.Container, block model.Block, userID string) error {
 	err := a.store.InsertBlock(c, &block, userID)
 	if err == nil {

server/app/blocks_test.go

@@ -62,3 +62,25 @@ func TestInsertBlock(t *testing.T) {
 		require.Error(t, err, "error")
 	})
 }
+
+func TestPatchBlocks(t *testing.T) {
+	th, tearDown := SetupTestHelper(t)
+	defer tearDown()
+
+	container := st.Container{
+		WorkspaceID: "0",
+	}
+	t.Run("patchBlocks success scenerio", func(t *testing.T) {
+		blockPatches := model.BlockPatchBatch{}
+		th.Store.EXPECT().PatchBlocks(gomock.Eq(container), gomock.Eq(&blockPatches), gomock.Eq("user-id-1")).Return(nil)
+		err := th.App.PatchBlocks(container, &blockPatches, "user-id-1")
+		require.NoError(t, err)
+	})
+
+	t.Run("patchBlocks error scenerio", func(t *testing.T) {
+		blockPatches := model.BlockPatchBatch{}
+		th.Store.EXPECT().PatchBlocks(gomock.Eq(container), gomock.Eq(&blockPatches), gomock.Eq("user-id-1")).Return(blockError{"error"})
+		err := th.App.PatchBlocks(container, &blockPatches, "user-id-1")
+		require.Error(t, err, "error")
+	})
+}

server/model/block.go

@@ -95,6 +95,16 @@ type BlockPatch struct {
 	DeletedFields []string `json:"deletedFields"`
 }
 
+// BlockPatchBatch is a batch of IDs and patches for modify blocks
+// swagger:model
+type BlockPatchBatch struct {
+	// The id's for of the blocks to patch
+	BlockIDs []string `json:"block_ids"`
+
+	// The BlockPatches to be applied
+	BlockPatches []BlockPatch `json:"block_patches"`
+}
+
 // Archive is an import / export archive.
 type Archive struct {
 	Version int64   `json:"version"`

server/services/store/mockstore/mockstore.go

@@ -494,6 +494,20 @@ func (mr *MockStoreMockRecorder) InsertBlock(arg0, arg1, arg2 interface{}) *gomo
 	return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "InsertBlock", reflect.TypeOf((*MockStore)(nil).InsertBlock), arg0, arg1, arg2)
 }
 
+// InsertBlocks mocks base method.
+func (m *MockStore) InsertBlocks(arg0 store.Container, arg1 []model.Block, arg2 string) error {
+	m.ctrl.T.Helper()
+	ret := m.ctrl.Call(m, "InsertBlocks", arg0, arg1, arg2)
+	ret0, _ := ret[0].(error)
+	return ret0
+}
+
+// InsertBlocks indicates an expected call of InsertBlocks.
+func (mr *MockStoreMockRecorder) InsertBlocks(arg0, arg1, arg2 interface{}) *gomock.Call {
+	mr.mock.ctrl.T.Helper()
+	return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "InsertBlocks", reflect.TypeOf((*MockStore)(nil).InsertBlocks), arg0, arg1, arg2)
+}
+
 // PatchBlock mocks base method.
 func (m *MockStore) PatchBlock(arg0 store.Container, arg1 string, arg2 *model.BlockPatch, arg3 string) error {
 	m.ctrl.T.Helper()
@@ -508,6 +522,20 @@ func (mr *MockStoreMockRecorder) PatchBlock(arg0, arg1, arg2, arg3 interface{})
 	return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "PatchBlock", reflect.TypeOf((*MockStore)(nil).PatchBlock), arg0, arg1, arg2, arg3)
 }
 
+// PatchBlocks mocks base method.
+func (m *MockStore) PatchBlocks(arg0 store.Container, arg1 *model.BlockPatchBatch, arg2 string) error {
+	m.ctrl.T.Helper()
+	ret := m.ctrl.Call(m, "PatchBlocks", arg0, arg1, arg2)
+	ret0, _ := ret[0].(error)
+	return ret0
+}
+
+// PatchBlocks indicates an expected call of PatchBlocks.
+func (mr *MockStoreMockRecorder) PatchBlocks(arg0, arg1, arg2 interface{}) *gomock.Call {
+	mr.mock.ctrl.T.Helper()
+	return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "PatchBlocks", reflect.TypeOf((*MockStore)(nil).PatchBlocks), arg0, arg1, arg2)
+}
+
 // RefreshSession mocks base method.
 func (m *MockStore) RefreshSession(arg0 *model.Session) error {
 	m.ctrl.T.Helper()

server/services/store/sqlstore/blocks.go

@@ -390,6 +390,26 @@ func (s *SQLStore) patchBlock(db sq.BaseRunner, c store.Container, blockID strin
 	return s.insertBlock(db, c, block, userID)
 }
 
+func (s *SQLStore) patchBlocks(db sq.BaseRunner, c store.Container, blockPatches *model.BlockPatchBatch, userID string) error {
+	for i, blockID := range blockPatches.BlockIDs {
+		err := s.patchBlock(db, c, blockID, &blockPatches.BlockPatches[i], userID)
+		if err != nil {
+			return err
+		}
+	}
+	return nil
+}
+
+func (s *SQLStore) insertBlocks(db sq.BaseRunner, c store.Container, blocks []model.Block, userID string) error {
+	for i := range blocks {
+		err := s.insertBlock(db, c, &blocks[i], userID)
+		if err != nil {
+			return err
+		}
+	}
+	return nil
+}
+
 func (s *SQLStore) deleteBlock(db sq.BaseRunner, c store.Container, blockID string, modifiedBy string) error {
 	block, err := s.getBlock(db, c, blockID)
 	if err != nil {

server/services/store/sqlstore/public_methods.go

@@ -208,6 +208,27 @@ func (s *SQLStore) InsertBlock(c store.Container, block *model.Block, userID str
 
 }
 
+func (s *SQLStore) InsertBlocks(c store.Container, blocks []model.Block, userID string) error {
+	tx, txErr := s.db.BeginTx(context.Background(), nil)
+	if txErr != nil {
+		return txErr
+	}
+	err := s.insertBlocks(tx, c, blocks, userID)
+	if err != nil {
+		if rollbackErr := tx.Rollback(); rollbackErr != nil {
+			s.logger.Error("transaction rollback error", mlog.Err(rollbackErr), mlog.String("methodName", "InsertBlocks"))
+		}
+		return err
+	}
+
+	if err := tx.Commit(); err != nil {
+		return err
+	}
+
+	return nil
+
+}
+
 func (s *SQLStore) PatchBlock(c store.Container, blockID string, blockPatch *model.BlockPatch, userID string) error {
 	tx, txErr := s.db.BeginTx(context.Background(), nil)
 	if txErr != nil {
@@ -229,6 +250,27 @@ func (s *SQLStore) PatchBlock(c store.Container, blockID string, blockPatch *mod
 
 }
 
+func (s *SQLStore) PatchBlocks(c store.Container, blockPatches *model.BlockPatchBatch, userID string) error {
+	tx, txErr := s.db.BeginTx(context.Background(), nil)
+	if txErr != nil {
+		return txErr
+	}
+	err := s.patchBlocks(tx, c, blockPatches, userID)
+	if err != nil {
+		if rollbackErr := tx.Rollback(); rollbackErr != nil {
+			s.logger.Error("transaction rollback error", mlog.Err(rollbackErr), mlog.String("methodName", "PatchBlocks"))
+		}
+		return err
+	}
+
+	if err := tx.Commit(); err != nil {
+		return err
+	}
+
+	return nil
+
+}
+
 func (s *SQLStore) RefreshSession(session *model.Session) error {
 	return s.refreshSession(s.db, session)
 

server/services/store/store.go

@@ -26,11 +26,15 @@ type Store interface {
 	// @withTransaction
 	InsertBlock(c Container, block *model.Block, userID string) error
 	// @withTransaction
+	InsertBlocks(c Container, blocks []model.Block, userID string) error
+	// @withTransaction
 	DeleteBlock(c Container, blockID string, modifiedBy string) error
 	GetBlockCountsByType() (map[string]int64, error)
 	GetBlock(c Container, blockID string) (*model.Block, error)
 	// @withTransaction
 	PatchBlock(c Container, blockID string, blockPatch *model.BlockPatch, userID string) error
+	// @withTransaction
+	PatchBlocks(c Container, blockPatches *model.BlockPatchBatch, userID string) error
 
 	Shutdown() error
 

server/services/store/storetests/blocks.go

@@ -26,11 +26,21 @@ func StoreTestBlocksStore(t *testing.T, setup func(t *testing.T) (store.Store, f
 		defer tearDown()
 		testInsertBlock(t, store, container)
 	})
+	t.Run("InsertBlocks", func(t *testing.T) {
+		store, tearDown := setup(t)
+		defer tearDown()
+		testInsertBlocks(t, store, container)
+	})
 	t.Run("PatchBlock", func(t *testing.T) {
 		store, tearDown := setup(t)
 		defer tearDown()
 		testPatchBlock(t, store, container)
 	})
+	t.Run("PatchBlocks", func(t *testing.T) {
+		store, tearDown := setup(t)
+		defer tearDown()
+		testPatchBlocks(t, store, container)
+	})
 	t.Run("DeleteBlock", func(t *testing.T) {
 		store, tearDown := setup(t)
 		defer tearDown()
@@ -189,6 +199,38 @@ func testInsertBlock(t *testing.T, store store.Store, container store.Container)
 	})
 }
 
+func testInsertBlocks(t *testing.T, store store.Store, container store.Container) {
+	userID := testUserID
+
+	blocks, errBlocks := store.GetAllBlocks(container)
+	require.NoError(t, errBlocks)
+	initialCount := len(blocks)
+
+	t.Run("invalid block", func(t *testing.T) {
+		validBlock := model.Block{
+			ID:         "id-test",
+			RootID:     "id-test",
+			ModifiedBy: userID,
+		}
+
+		invalidBlock := model.Block{
+			ID:         "id-test",
+			RootID:     "",
+			ModifiedBy: userID,
+		}
+
+		newBlocks := []model.Block{validBlock, invalidBlock}
+
+		err := store.InsertBlocks(container, newBlocks, "user-id-1")
+		require.Error(t, err)
+
+		blocks, err := store.GetAllBlocks(container)
+		require.NoError(t, err)
+		// no blocks should have been inserted
+		require.Len(t, blocks, initialCount)
+	})
+}
+
 func testPatchBlock(t *testing.T, store store.Store, container store.Container) {
 	userID := testUserID
 
@@ -309,6 +351,70 @@ func testPatchBlock(t *testing.T, store store.Store, container store.Container)
 	})
 }
 
+func testPatchBlocks(t *testing.T, store store.Store, container store.Container) {
+	block := model.Block{
+		ID:     "id-test",
+		RootID: "id-test",
+		Title:  "oldTitle",
+	}
+
+	block2 := model.Block{
+		ID:     "id-test2",
+		RootID: "id-test2",
+		Title:  "oldTitle2",
+	}
+
+	insertBlocks := []model.Block{block, block2}
+	err := store.InsertBlocks(container, insertBlocks, "user-id-1")
+	require.NoError(t, err)
+
+	t.Run("successful updated existing blocks", func(t *testing.T) {
+		title := "updatedTitle"
+		blockPatch := model.BlockPatch{
+			Title: &title,
+		}
+
+		blockPatch2 := model.BlockPatch{
+			Title: &title,
+		}
+
+		blockIds := []string{"id-test", "id-test2"}
+		blockPatches := []model.BlockPatch{blockPatch, blockPatch2}
+
+		err := store.PatchBlocks(container, &model.BlockPatchBatch{BlockIDs: blockIds, BlockPatches: blockPatches}, "user-id-1")
+		require.NoError(t, err)
+
+		retrievedBlock, err := store.GetBlock(container, "id-test")
+		require.NoError(t, err)
+		require.Equal(t, title, retrievedBlock.Title)
+
+		retrievedBlock2, err := store.GetBlock(container, "id-test2")
+		require.NoError(t, err)
+		require.Equal(t, title, retrievedBlock2.Title)
+	})
+
+	t.Run("invalid block id, nothing updated existing blocks", func(t *testing.T) {
+		title := "Another Title"
+		blockPatch := model.BlockPatch{
+			Title: &title,
+		}
+
+		blockPatch2 := model.BlockPatch{
+			Title: &title,
+		}
+
+		blockIds := []string{"id-test", "invalid id"}
+		blockPatches := []model.BlockPatch{blockPatch, blockPatch2}
+
+		err := store.PatchBlocks(container, &model.BlockPatchBatch{BlockIDs: blockIds, BlockPatches: blockPatches}, "user-id-1")
+		require.Error(t, err)
+
+		retrievedBlock, err := store.GetBlock(container, "id-test")
+		require.NoError(t, err)
+		require.NotEqual(t, title, retrievedBlock.Title)
+	})
+}
+
 var (
 	subtreeSampleBlocks = []model.Block{
 		{

webapp/src/mutator.ts

@@ -80,14 +80,10 @@ class Mutator {
 
         return undoManager.perform(
             async () => {
-                await Promise.all(
+                await octoClient.patchBlocks(newBlocks, updatePatches)
-                    updatePatches.map((patch, i) => octoClient.patchBlock(newBlocks[i].id, patch)),
-                )
             },
             async () => {
-                await Promise.all(
+                await octoClient.patchBlocks(newBlocks, undoPatches)
-                    undoPatches.map((patch, i) => octoClient.patchBlock(newBlocks[i].id, patch)),
-                )
             },
             description,
             this.undoGroupId,

webapp/src/octoClient.ts

@@ -268,6 +268,20 @@ class OctoClient {
         })
     }
 
+    async patchBlocks(blocks: Block[], blockPatches: BlockPatch[]): Promise<Response> {
+        Utils.log(`patchBlocks: ${blocks.length} blocks`)
+        const blockIds = blocks.map((block) => block.id)
+        const body = JSON.stringify({block_ids: blockIds, block_patches: blockPatches})
+
+        const path = this.getBaseURL() + this.workspacePath() + '/blocks'
+        const response = fetch(path, {
+            method: 'PATCH',
+            headers: this.headers(),
+            body,
+        })
+        return response
+    }
+
     async deleteBlock(blockId: string): Promise<Response> {
         Utils.log(`deleteBlock: ${blockId}`)
         return fetch(this.getBaseURL() + this.workspacePath() + `/blocks/${encodeURIComponent(blockId)}`, {