Updated update category-board mapping method to not rely on DB returning affected rows (#2840)

* Updated update category-board mapping method to not rely on DB returning affected rows

* Wrapped into transaction

server/services/store/sqlstore/category_boards.go

@@ -2,7 +2,6 @@ package sqlstore
 
 import (
 	"database/sql"
-	"errors"
 
 	sq "github.com/Masterminds/squirrel"
 	"github.com/mattermost/focalboard/server/model"
@@ -11,10 +10,6 @@ import (
 	"github.com/mattermost/mattermost-server/v6/shared/mlog"
 )
 
-var (
-	errDuplicateCategoryEntries = errors.New("duplicate entries found for user-board-category mapping")
-)
-
 func (s *SQLStore) getUserCategoryBoards(db sq.BaseRunner, userID, teamID string) ([]model.CategoryBoards, error) {
 	categories, err := s.getUserCategories(db, userID, teamID)
 	if err != nil {
@@ -58,25 +53,18 @@ func (s *SQLStore) getCategoryBoardAttributes(db sq.BaseRunner, categoryID strin
 }
 
 func (s *SQLStore) addUpdateCategoryBoard(db sq.BaseRunner, userID, categoryID, boardID string) error {
-	if categoryID == "0" {
-		return s.deleteUserCategoryBoard(db, userID, boardID)
-	}
-
-	rowsAffected, err := s.updateUserCategoryBoard(db, userID, boardID, categoryID)
-	if err != nil {
+	if err := s.deleteUserCategoryBoard(db, userID, boardID); err != nil {
 		return err
 	}
 
-	if rowsAffected > 1 {
-		return errDuplicateCategoryEntries
+	if categoryID == "0" {
+		// category ID "0" means user wants to move board out of
+		// the custom category. Deleting the user-board-category
+		// mapping achieves this.
+		return nil
 	}
 
-	if rowsAffected == 0 {
-		// user-block mapping didn't already exist. So we'll create a new entry
-		return s.addUserCategoryBoard(db, userID, categoryID, boardID)
-	}
-
-	return nil
+	return s.addUserCategoryBoard(db, userID, categoryID, boardID)
 }
 
 /*
@@ -102,31 +90,6 @@ func (s *SQLStore) userCategoryBoardExists(db sq.BaseRunner, userID, teamID, cat
 }
 */
 
-func (s *SQLStore) updateUserCategoryBoard(db sq.BaseRunner, userID, boardID, categoryID string) (int64, error) {
-	result, err := s.getQueryBuilder(db).
-		Update(s.tablePrefix+"category_boards").
-		Set("category_id", categoryID).
-		Set("delete_at", 0).
-		Where(sq.Eq{
-			"board_id": boardID,
-			"user_id":  userID,
-		}).
-		Exec()
-
-	if err != nil {
-		s.logger.Error("updateUserCategoryBoard error", mlog.Err(err))
-		return 0, err
-	}
-
-	rowsAffected, err := result.RowsAffected()
-	if err != nil {
-		s.logger.Error("updateUserCategoryBoard affected row count error", mlog.Err(err))
-		return 0, err
-	}
-
-	return rowsAffected, nil
-}
-
 func (s *SQLStore) addUserCategoryBoard(db sq.BaseRunner, userID, categoryID, boardID string) error {
 	_, err := s.getQueryBuilder(db).
 		Insert(s.tablePrefix+"category_boards").

server/services/store/sqlstore/public_methods.go

@@ -23,7 +23,26 @@ import (
 )
 
 func (s *SQLStore) AddUpdateCategoryBoard(userID string, categoryID string, blockID string) error {
-	return s.addUpdateCategoryBoard(s.db, userID, categoryID, blockID)
+	if s.dbType == model.SqliteDBType {
+		return s.addUpdateCategoryBoard(s.db, userID, categoryID, blockID)
+	}
+	tx, txErr := s.db.BeginTx(context.Background(), nil)
+	if txErr != nil {
+		return txErr
+	}
+	err := s.addUpdateCategoryBoard(tx, userID, categoryID, blockID)
+	if err != nil {
+		if rollbackErr := tx.Rollback(); rollbackErr != nil {
+			s.logger.Error("transaction rollback error", mlog.Err(rollbackErr), mlog.String("methodName", "AddUpdateCategoryBoard"))
+		}
+		return err
+	}
+
+	if err := tx.Commit(); err != nil {
+		return err
+	}
+
+	return nil
 
 }
 

server/services/store/store.go

@@ -113,6 +113,8 @@ type Store interface {
 	DeleteCategory(categoryID, userID, teamID string) error
 
 	GetUserCategoryBoards(userID, teamID string) ([]model.CategoryBoards, error)
+
+	// @withTransaction
 	AddUpdateCategoryBoard(userID, categoryID, blockID string) error
 
 	CreateSubscription(sub *model.Subscription) (*model.Subscription, error)

webapp/src/components/sidebar/sidebarBoardItem.tsx

@@ -75,7 +75,9 @@ const SidebarBoardItem = (props: Props) => {
                 icon={category.id === props.categoryBoards.id ? <Check/> : <Folder/>}
                 onClick={async (toCategoryID) => {
                     const fromCategoryID = props.categoryBoards.id
-                    await mutator.moveBoardToCategory(teamID, boardID, toCategoryID, fromCategoryID)
+                    if (fromCategoryID !== toCategoryID) {
+                        await mutator.moveBoardToCategory(teamID, boardID, toCategoryID, fromCategoryID)
+                    }
                 }}
             />
         ))