Adding sever side undelete endpoint (#2222)

* Adding sever side undelete endpoint

* Removing long lines golangci-lint errors

* Fixing linter errors

* Fixing a test problem

* Fixing tests

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

server/.golangci.yml

@@ -14,6 +14,8 @@ linters-settings:
       - fieldalignment
   lll:
     line-length: 150
+  dupl:
+    threshold: 200
   revive:
     enableAllRules: true
     rules:

server/api/api.go

@@ -72,6 +72,7 @@ func (a *API) RegisterRoutes(r *mux.Router) {
 	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}/undelete", a.sessionRequired(a.handleUndeleteBlock)).Methods("POST")
 	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")
 
@@ -633,6 +634,64 @@ func (a *API) handleDeleteBlock(w http.ResponseWriter, r *http.Request) {
 	auditRec.Success()
 }
 
+func (a *API) handleUndeleteBlock(w http.ResponseWriter, r *http.Request) {
+	// swagger:operation POST /api/v1/workspaces/{workspaceID}/blocks/{blockID}/undelete undeleteBlock
+	//
+	// Undeletes a block
+	//
+	// ---
+	// produces:
+	// - application/json
+	// parameters:
+	// - name: workspaceID
+	//   in: path
+	//   description: Workspace ID
+	//   required: true
+	//   type: string
+	// - name: blockID
+	//   in: path
+	//   description: ID of block to undelete
+	//   required: true
+	//   type: string
+	// 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
+
+	vars := mux.Vars(r)
+	blockID := vars["blockID"]
+
+	container, err := a.getContainer(r)
+	if err != nil {
+		a.noContainerErrorResponse(w, r.URL.Path, err)
+		return
+	}
+
+	auditRec := a.makeAuditRecord(r, "undeleteBlock", audit.Fail)
+	defer a.audit.LogRecord(audit.LevelModify, auditRec)
+	auditRec.AddMeta("blockID", blockID)
+
+	err = a.app.UndeleteBlock(*container, blockID, userID)
+	if err != nil {
+		a.errorResponse(w, r.URL.Path, http.StatusInternalServerError, "", err)
+		return
+	}
+
+	a.logger.Debug("UNDELETE Block", mlog.String("blockID", blockID))
+	jsonStringResponse(w, http.StatusOK, "{}")
+
+	auditRec.Success()
+}
+
 func (a *API) handlePatchBlock(w http.ResponseWriter, r *http.Request) {
 	// swagger:operation PATCH /api/v1/workspaces/{workspaceID}/blocks/{blockID} patchBlock
 	//

server/app/blocks.go

@@ -233,6 +233,41 @@ func (a *App) DeleteBlock(c store.Container, blockID string, modifiedBy string)
 	return nil
 }
 
+func (a *App) UndeleteBlock(c store.Container, blockID string, modifiedBy string) error {
+	blocks, err := a.store.GetBlockHistory(c, blockID, model.QueryBlockHistoryOptions{Limit: 1, Descending: true})
+	if err != nil {
+		return err
+	}
+
+	if len(blocks) == 0 {
+		// deleting non-existing block not considered an error
+		return nil
+	}
+
+	err = a.store.UndeleteBlock(c, blockID, modifiedBy)
+	if err != nil {
+		return err
+	}
+
+	block, err := a.store.GetBlock(c, blockID)
+	if err != nil {
+		return err
+	}
+
+	if block == nil {
+		a.logger.Error("Error loading the block after undelete, not propagating through websockets or notifications")
+		return nil
+	}
+
+	a.wsAdapter.BroadcastBlockChange(c.WorkspaceID, *block)
+	a.metrics.IncrementBlocksInserted(1)
+	go func() {
+		a.webhook.NotifyUpdate(*block)
+		a.notifyBlockChanged(notify.Add, c, block, nil, modifiedBy)
+	}()
+	return nil
+}
+
 func (a *App) GetBlockCountsByType() (map[string]int64, error) {
 	return a.store.GetBlockCountsByType()
 }

server/app/blocks_test.go

@@ -84,3 +84,71 @@ func TestPatchBlocks(t *testing.T) {
 		require.Error(t, err, "error")
 	})
 }
+
+func TestDeleteBlock(t *testing.T) {
+	th, tearDown := SetupTestHelper(t)
+	defer tearDown()
+
+	container := st.Container{
+		WorkspaceID: "0",
+	}
+
+	t.Run("success scenerio", func(t *testing.T) {
+		block := model.Block{
+			ID: "block-id",
+		}
+		th.Store.EXPECT().GetBlock(gomock.Eq(container), gomock.Eq("block-id")).Return(&block, nil)
+		th.Store.EXPECT().DeleteBlock(gomock.Eq(container), gomock.Eq("block-id"), gomock.Eq("user-id-1")).Return(nil)
+		err := th.App.DeleteBlock(container, "block-id", "user-id-1")
+		require.NoError(t, err)
+	})
+
+	t.Run("error scenerio", func(t *testing.T) {
+		block := model.Block{
+			ID: "block-id",
+		}
+		th.Store.EXPECT().GetBlock(gomock.Eq(container), gomock.Eq("block-id")).Return(&block, nil)
+		th.Store.EXPECT().DeleteBlock(gomock.Eq(container), gomock.Eq("block-id"), gomock.Eq("user-id-1")).Return(blockError{"error"})
+		err := th.App.DeleteBlock(container, "block-id", "user-id-1")
+		require.Error(t, err, "error")
+	})
+}
+
+func TestUndeleteBlock(t *testing.T) {
+	th, tearDown := SetupTestHelper(t)
+	defer tearDown()
+
+	container := st.Container{
+		WorkspaceID: "0",
+	}
+
+	t.Run("success scenerio", func(t *testing.T) {
+		block := model.Block{
+			ID: "block-id",
+		}
+		th.Store.EXPECT().GetBlockHistory(
+			gomock.Eq(container),
+			gomock.Eq("block-id"),
+			gomock.Eq(model.QueryBlockHistoryOptions{Limit: 1, Descending: true}),
+		).Return([]model.Block{block}, nil)
+		th.Store.EXPECT().UndeleteBlock(gomock.Eq(container), gomock.Eq("block-id"), gomock.Eq("user-id-1")).Return(nil)
+		th.Store.EXPECT().GetBlock(gomock.Eq(container), gomock.Eq("block-id")).Return(&block, nil)
+		err := th.App.UndeleteBlock(container, "block-id", "user-id-1")
+		require.NoError(t, err)
+	})
+
+	t.Run("error scenerio", func(t *testing.T) {
+		block := model.Block{
+			ID: "block-id",
+		}
+		th.Store.EXPECT().GetBlockHistory(
+			gomock.Eq(container),
+			gomock.Eq("block-id"),
+			gomock.Eq(model.QueryBlockHistoryOptions{Limit: 1, Descending: true}),
+		).Return([]model.Block{block}, nil)
+		th.Store.EXPECT().UndeleteBlock(gomock.Eq(container), gomock.Eq("block-id"), gomock.Eq("user-id-1")).Return(blockError{"error"})
+		th.Store.EXPECT().GetBlock(gomock.Eq(container), gomock.Eq("block-id")).Return(&block, nil)
+		err := th.App.UndeleteBlock(container, "block-id", "user-id-1")
+		require.Error(t, err, "error")
+	})
+}

server/client/client.go

@@ -204,6 +204,16 @@ func (c *Client) DeleteBlock(blockID string) (bool, *Response) {
 	return true, BuildResponse(r)
 }
 
+func (c *Client) UndeleteBlock(blockID string) (bool, *Response) {
+	r, err := c.DoAPIPost(c.GetBlockRoute(blockID)+"/undelete", "")
+	if err != nil {
+		return false, BuildErrorResponse(r, err)
+	}
+	defer closeBody(r)
+
+	return true, BuildResponse(r)
+}
+
 func (c *Client) GetSubtree(blockID string) ([]model.Block, *Response) {
 	r, err := c.DoAPIGet(c.GetSubtreeRoute(blockID), "")
 	if err != nil {

server/integrationtests/blocks_test.go

@@ -316,6 +316,71 @@ func TestDeleteBlock(t *testing.T) {
 	})
 }
 
+func TestUndeleteBlock(t *testing.T) {
+	th := SetupTestHelper().InitBasic()
+	defer th.TearDown()
+
+	blocks, resp := th.Client.GetBlocks()
+	require.NoError(t, resp.Error)
+	initialCount := len(blocks)
+
+	var blockID string
+	t.Run("Create a block", func(t *testing.T) {
+		initialID := utils.NewID(utils.IDTypeBlock)
+		block := model.Block{
+			ID:       initialID,
+			RootID:   initialID,
+			CreateAt: 1,
+			UpdateAt: 1,
+			Type:     model.TypeBoard,
+			Title:    "New title",
+		}
+
+		newBlocks, resp := th.Client.InsertBlocks([]model.Block{block})
+		require.NoError(t, resp.Error)
+		require.Len(t, newBlocks, 1)
+		require.NotZero(t, newBlocks[0].ID)
+		require.NotEqual(t, initialID, newBlocks[0].ID)
+		blockID = newBlocks[0].ID
+
+		blocks, resp := th.Client.GetBlocks()
+		require.NoError(t, resp.Error)
+		require.Len(t, blocks, initialCount+1)
+
+		blockIDs := make([]string, len(blocks))
+		for i, b := range blocks {
+			blockIDs[i] = b.ID
+		}
+		require.Contains(t, blockIDs, blockID)
+	})
+
+	t.Run("Delete a block", func(t *testing.T) {
+		// this avoids triggering uniqueness constraint of
+		// id,insert_at on block history
+		time.Sleep(10 * time.Millisecond)
+
+		_, resp := th.Client.DeleteBlock(blockID)
+		require.NoError(t, resp.Error)
+
+		blocks, resp := th.Client.GetBlocks()
+		require.NoError(t, resp.Error)
+		require.Len(t, blocks, initialCount)
+	})
+
+	t.Run("Undelete a block", func(t *testing.T) {
+		// this avoids triggering uniqueness constraint of
+		// id,insert_at on block history
+		time.Sleep(10 * time.Millisecond)
+
+		_, resp := th.Client.UndeleteBlock(blockID)
+		require.NoError(t, resp.Error)
+
+		blocks, resp := th.Client.GetBlocks()
+		require.NoError(t, resp.Error)
+		require.Len(t, blocks, initialCount+1)
+	})
+}
+
 func TestGetSubtree(t *testing.T) {
 	t.Skip("TODO: fix flaky test")
 

server/services/store/mockstore/mockstore.go

@@ -802,6 +802,20 @@ func (mr *MockStoreMockRecorder) Shutdown() *gomock.Call {
 	return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Shutdown", reflect.TypeOf((*MockStore)(nil).Shutdown))
 }
 
+// UndeleteBlock mocks base method.
+func (m *MockStore) UndeleteBlock(arg0 store.Container, arg1, arg2 string) error {
+	m.ctrl.T.Helper()
+	ret := m.ctrl.Call(m, "UndeleteBlock", arg0, arg1, arg2)
+	ret0, _ := ret[0].(error)
+	return ret0
+}
+
+// UndeleteBlock indicates an expected call of UndeleteBlock.
+func (mr *MockStoreMockRecorder) UndeleteBlock(arg0, arg1, arg2 interface{}) *gomock.Call {
+	mr.mock.ctrl.T.Helper()
+	return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "UndeleteBlock", reflect.TypeOf((*MockStore)(nil).UndeleteBlock), arg0, arg1, arg2)
+}
+
 // UpdateSession mocks base method.
 func (m *MockStore) UpdateSession(arg0 *model.Session) error {
 	m.ctrl.T.Helper()

server/services/store/sqlstore/blocks.go

@@ -516,6 +516,76 @@ func (s *SQLStore) deleteBlock(db sq.BaseRunner, c store.Container, blockID stri
 
 	return nil
 }
+
+func (s *SQLStore) undeleteBlock(db sq.BaseRunner, c store.Container, blockID string, modifiedBy string) error {
+	blocks, err := s.getBlockHistory(db, c, blockID, model.QueryBlockHistoryOptions{Limit: 1, Descending: true})
+	if err != nil {
+		return err
+	}
+
+	if len(blocks) == 0 {
+		return nil // deleting non-exiting block is not considered an error (for now)
+	}
+	block := blocks[0]
+
+	if block.DeleteAt == 0 {
+		return nil // undeleting not deleted block is not considered an error (for now)
+	}
+
+	fieldsJSON, err := json.Marshal(block.Fields)
+	if err != nil {
+		return err
+	}
+
+	now := utils.GetMillis()
+	columns := []string{
+		"workspace_id",
+		"id",
+		"parent_id",
+		s.escapeField("schema"),
+		"type",
+		"title",
+		"fields",
+		"root_id",
+		"modified_by",
+		"create_at",
+		"update_at",
+		"delete_at",
+		"created_by",
+	}
+
+	values := []interface{}{
+		c.WorkspaceID,
+		block.ID,
+		block.ParentID,
+		block.Schema,
+		block.Type,
+		block.Title,
+		fieldsJSON,
+		block.RootID,
+		modifiedBy,
+		block.CreateAt,
+		now,
+		0,
+		block.CreatedBy,
+	}
+	insertHistoryQuery := s.getQueryBuilder(db).Insert(s.tablePrefix + "blocks_history").
+		Columns(columns...).
+		Values(values...)
+	insertQuery := s.getQueryBuilder(db).Insert(s.tablePrefix + "blocks").
+		Columns(columns...).
+		Values(values...)
+
+	if _, err := insertHistoryQuery.Exec(); err != nil {
+		return err
+	}
+
+	if _, err := insertQuery.Exec(); err != nil {
+		return err
+	}
+
+	return nil
+}
 func (s *SQLStore) getBlockCountsByType(db sq.BaseRunner) (map[string]int64, error) {
 	query := s.getQueryBuilder(db).
 		Select(

server/services/store/sqlstore/public_methods.go

@@ -352,6 +352,27 @@ func (s *SQLStore) SetSystemSetting(key string, value string) error {
 
 }
 
+func (s *SQLStore) UndeleteBlock(c store.Container, blockID string, modifiedBy string) error {
+	tx, txErr := s.db.BeginTx(context.Background(), nil)
+	if txErr != nil {
+		return txErr
+	}
+	err := s.undeleteBlock(tx, c, blockID, modifiedBy)
+	if err != nil {
+		if rollbackErr := tx.Rollback(); rollbackErr != nil {
+			s.logger.Error("transaction rollback error", mlog.Err(rollbackErr), mlog.String("methodName", "UndeleteBlock"))
+		}
+		return err
+	}
+
+	if err := tx.Commit(); err != nil {
+		return err
+	}
+
+	return nil
+
+}
+
 func (s *SQLStore) UpdateSession(session *model.Session) error {
 	return s.updateSession(s.db, session)
 

server/services/store/store.go

@@ -33,6 +33,8 @@ type Store interface {
 	InsertBlocks(c Container, blocks []model.Block, userID string) error
 	// @withTransaction
 	DeleteBlock(c Container, blockID string, modifiedBy string) error
+	// @withTransaction
+	UndeleteBlock(c Container, blockID string, modifiedBy string) error
 	GetBlockCountsByType() (map[string]int64, error)
 	GetBlock(c Container, blockID string) (*model.Block, error)
 	// @withTransaction

server/services/store/storetests/blocks.go

@@ -46,6 +46,11 @@ func StoreTestBlocksStore(t *testing.T, setup func(t *testing.T) (store.Store, f
 		defer tearDown()
 		testDeleteBlock(t, store, container)
 	})
+	t.Run("UndeleteBlock", func(t *testing.T) {
+		store, tearDown := setup(t)
+		defer tearDown()
+		testUndeleteBlock(t, store, container)
+	})
 	t.Run("GetSubTree2", func(t *testing.T) {
 		store, tearDown := setup(t)
 		defer tearDown()
@@ -381,6 +386,7 @@ func testPatchBlocks(t *testing.T, store store.Store, container store.Container)
 		blockIds := []string{"id-test", "id-test2"}
 		blockPatches := []model.BlockPatch{blockPatch, blockPatch2}
 
+		time.Sleep(1 * time.Millisecond)
 		err := store.PatchBlocks(container, &model.BlockPatchBatch{BlockIDs: blockIds, BlockPatches: blockPatches}, "user-id-1")
 		require.NoError(t, err)
 
@@ -634,6 +640,96 @@ func testDeleteBlock(t *testing.T, store store.Store, container store.Container)
 	})
 }
 
+func testUndeleteBlock(t *testing.T, store store.Store, container store.Container) {
+	userID := testUserID
+
+	blocks, err := store.GetAllBlocks(container)
+	require.NoError(t, err)
+	initialCount := len(blocks)
+
+	blocksToInsert := []model.Block{
+		{
+			ID:         "block1",
+			RootID:     "block1",
+			ModifiedBy: userID,
+		},
+		{
+			ID:         "block2",
+			RootID:     "block2",
+			ModifiedBy: userID,
+		},
+		{
+			ID:         "block3",
+			RootID:     "block3",
+			ModifiedBy: userID,
+		},
+	}
+	InsertBlocks(t, store, container, blocksToInsert, "user-id-1")
+	defer DeleteBlocks(t, store, container, blocksToInsert, "test")
+
+	blocks, err = store.GetAllBlocks(container)
+	require.NoError(t, err)
+	require.Len(t, blocks, initialCount+3)
+
+	t.Run("exiting id", func(t *testing.T) {
+		// Wait for not colliding the ID+insert_at key
+		time.Sleep(1 * time.Millisecond)
+		err := store.DeleteBlock(container, "block1", userID)
+		require.NoError(t, err)
+
+		block, err := store.GetBlock(container, "block1")
+		require.NoError(t, err)
+		require.Nil(t, block)
+
+		err = store.UndeleteBlock(container, "block1", userID)
+		require.NoError(t, err)
+
+		block, err = store.GetBlock(container, "block1")
+		require.NoError(t, err)
+		require.NotNil(t, block)
+	})
+
+	t.Run("exiting id multiple times", func(t *testing.T) {
+		// Wait for not colliding the ID+insert_at key
+		time.Sleep(1 * time.Millisecond)
+		err := store.DeleteBlock(container, "block1", userID)
+		require.NoError(t, err)
+
+		block, err := store.GetBlock(container, "block1")
+		require.NoError(t, err)
+		require.Nil(t, block)
+
+		// Wait for not colliding the ID+insert_at key
+		time.Sleep(1 * time.Millisecond)
+		err = store.UndeleteBlock(container, "block1", userID)
+		require.NoError(t, err)
+
+		block, err = store.GetBlock(container, "block1")
+		require.NoError(t, err)
+		require.NotNil(t, block)
+
+		// Wait for not colliding the ID+insert_at key
+		time.Sleep(1 * time.Millisecond)
+		err = store.UndeleteBlock(container, "block1", userID)
+		require.NoError(t, err)
+
+		block, err = store.GetBlock(container, "block1")
+		require.NoError(t, err)
+		require.NotNil(t, block)
+	})
+
+	t.Run("from not existing id", func(t *testing.T) {
+		// Wait for not colliding the ID+insert_at key
+		time.Sleep(1 * time.Millisecond)
+		err := store.UndeleteBlock(container, "not-exists", userID)
+		require.NoError(t, err)
+
+		block, err := store.GetBlock(container, "not-exists")
+		require.NoError(t, err)
+		require.Nil(t, block)
+	})
+}
+
 func testGetBlocks(t *testing.T, store store.Store, container store.Container) {
 	blocks, err := store.GetAllBlocks(container)
 	require.NoError(t, err)

webapp/src/mutator.ts

@@ -163,7 +163,7 @@ class Mutator {
                 await octoClient.deleteBlock(block.id)
             },
             async () => {
-                await octoClient.insertBlock(block)
+                await octoClient.undeleteBlock(block.id)
                 await afterUndo?.()
             },
             actualDescription,

webapp/src/octoClient.ts

@@ -288,6 +288,14 @@ class OctoClient {
         })
     }
 
+    async undeleteBlock(blockId: string): Promise<Response> {
+        Utils.log(`undeleteBlock: ${blockId}`)
+        return fetch(this.getBaseURL() + this.workspacePath() + `/blocks/${encodeURIComponent(blockId)}/undelete`, {
+            method: 'POST',
+            headers: this.headers(),
+        })
+    }
+
     async followBlock(blockId: string, blockType: string, userId: string): Promise<Response> {
         const body: Subscription = {
             blockType,