From ab02e98831a249d6a16f0f0a06f404d184f44df0 Mon Sep 17 00:00:00 2001 From: Harshil Sharma <18575143+harshilsharma63@users.noreply.github.com> Date: Wed, 10 Aug 2022 16:31:34 +0530 Subject: [PATCH] Duplicate board retain category (#3395) * Converted synthetic membership to natuaral if needed * Creating duplicated board to same category as parent board * fixed tests * fixed an incorrect API path in client * Added logic to prevent jumping of new board between categories * fixed lint error --- server/app/boards.go | 36 +++++++ server/app/onboarding_test.go | 2 + server/client/client.go | 32 ++++++ server/integrationtests/board_test.go | 102 ++++++++++++++++++ server/model/category.go | 8 ++ .../components/sidebar/sidebarBoardItem.tsx | 22 +++- 6 files changed, 201 insertions(+), 1 deletion(-) diff --git a/server/app/boards.go b/server/app/boards.go index a66686b1f..a3060ac22 100644 --- a/server/app/boards.go +++ b/server/app/boards.go @@ -145,6 +145,36 @@ func (a *App) getBoardDescendantModifiedInfo(boardID string, latest bool) (int64 return timestamp, modifiedBy, nil } +func (a *App) setBoardCategoryFromSource(sourceBoardID, destinationBoardID, userID, teamID string) error { + // find source board's category ID for the user + userCategoryBoards, err := a.GetUserCategoryBoards(userID, teamID) + if err != nil { + return err + } + + var destinationCategoryID string + + for _, categoryBoard := range userCategoryBoards { + for _, boardID := range categoryBoard.BoardIDs { + if boardID == sourceBoardID { + // category found! + destinationCategoryID = categoryBoard.ID + break + } + } + } + + // if source board is not mapped to a category for this user, + // then we have nothing more to do. + if destinationCategoryID == "" { + return nil + } + + // now that we have source board's category, + // we send destination board to the same category + return a.AddUpdateUserCategoryBoard(teamID, userID, destinationCategoryID, destinationBoardID) +} + func (a *App) DuplicateBoard(boardID, userID, toTeam string, asTemplate bool) (*model.BoardsAndBlocks, []*model.BoardMember, error) { bab, members, err := a.store.DuplicateBoard(boardID, userID, toTeam, asTemplate) if err != nil { @@ -156,6 +186,12 @@ func (a *App) DuplicateBoard(boardID, userID, toTeam string, asTemplate bool) (* a.logger.Error("Could not copy files while duplicating board", mlog.String("BoardID", boardID), mlog.Err(err)) } + for _, board := range bab.Boards { + if categoryErr := a.setBoardCategoryFromSource(boardID, board.ID, userID, board.TeamID); categoryErr != nil { + return nil, nil, categoryErr + } + } + // bab.Blocks now has updated file ids for any blocks containing files. We need to store them. blockIDs := make([]string, 0) blockPatches := make([]model.BlockPatch, 0) diff --git a/server/app/onboarding_test.go b/server/app/onboarding_test.go index bc1b27eb6..eb39679b7 100644 --- a/server/app/onboarding_test.go +++ b/server/app/onboarding_test.go @@ -50,6 +50,7 @@ func TestPrepareOnboardingTour(t *testing.T) { } th.Store.EXPECT().PatchUserProps(userID, userPropPatch).Return(nil) + th.Store.EXPECT().GetUserCategoryBoards(userID, "0").Return([]model.CategoryBoards{}, nil) teamID, boardID, err := th.App.PrepareOnboardingTour(userID, teamID) assert.NoError(t, err) @@ -86,6 +87,7 @@ func TestCreateWelcomeBoard(t *testing.T) { } newType := model.BoardTypePrivate th.Store.EXPECT().PatchBoard("board_id_1", &model.BoardPatch{Type: &newType}, "user_id_1").Return(&privateWelcomeBoard, nil) + th.Store.EXPECT().GetUserCategoryBoards(userID, "0") boardID, err := th.App.createWelcomeBoard(userID, teamID) assert.Nil(t, err) diff --git a/server/client/client.go b/server/client/client.go index 10d21476b..8ce0e74d9 100644 --- a/server/client/client.go +++ b/server/client/client.go @@ -348,6 +348,38 @@ func (c *Client) CreateBoardsAndBlocks(bab *model.BoardsAndBlocks) (*model.Board return model.BoardsAndBlocksFromJSON(r.Body), BuildResponse(r) } +func (c *Client) CreateCategory(category model.Category) (*model.Category, *Response) { + r, err := c.DoAPIPost(c.GetTeamRoute(category.TeamID)+"/categories", toJSON(category)) + if err != nil { + return nil, BuildErrorResponse(r, err) + } + defer closeBody(r) + + return model.CategoryFromJSON(r.Body), BuildResponse(r) +} + +func (c *Client) UpdateCategoryBoard(teamID, categoryID, boardID string) *Response { + r, err := c.DoAPIPost(fmt.Sprintf("%s/categories/%s/boards/%s", c.GetTeamRoute(teamID), categoryID, boardID), "") + if err != nil { + return BuildErrorResponse(r, err) + } + defer closeBody(r) + + return BuildResponse(r) +} + +func (c *Client) GetUserCategoryBoards(teamID string) ([]model.CategoryBoards, *Response) { + r, err := c.DoAPIGet(c.GetTeamRoute(teamID)+"/categories", "") + if err != nil { + return nil, BuildErrorResponse(r, err) + } + defer closeBody(r) + + var categoryBoards []model.CategoryBoards + _ = json.NewDecoder(r.Body).Decode(&categoryBoards) + return categoryBoards, BuildResponse(r) +} + func (c *Client) PatchBoardsAndBlocks(pbab *model.PatchBoardsAndBlocks) (*model.BoardsAndBlocks, *Response) { r, err := c.DoAPIPatch(c.GetBoardsAndBlocksRoute(), toJSON(pbab)) if err != nil { diff --git a/server/integrationtests/board_test.go b/server/integrationtests/board_test.go index 4219edce9..cecfe4bd3 100644 --- a/server/integrationtests/board_test.go +++ b/server/integrationtests/board_test.go @@ -1936,6 +1936,108 @@ func TestDuplicateBoard(t *testing.T) { require.Equal(t, duplicateBoard.ID, members[0].BoardID) require.True(t, members[0].SchemeAdmin) }) + + t.Run("create and duplicate public board from a custom category", func(t *testing.T) { + th := SetupTestHelper(t).InitBasic() + defer th.TearDown() + + me := th.GetUser1() + teamID := testTeamID + + category := model.Category{ + Name: "My Category", + UserID: me.ID, + TeamID: teamID, + } + createdCategory, resp := th.Client.CreateCategory(category) + th.CheckOK(resp) + require.NoError(t, resp.Error) + require.NotNil(t, createdCategory) + require.Equal(t, "My Category", createdCategory.Name) + require.Equal(t, me.ID, createdCategory.UserID) + require.Equal(t, teamID, createdCategory.TeamID) + + title := "Public board" + newBoard := &model.Board{ + Title: title, + Type: model.BoardTypeOpen, + TeamID: teamID, + } + board, resp := th.Client.CreateBoard(newBoard) + th.CheckOK(resp) + require.NoError(t, resp.Error) + require.NotNil(t, board) + require.NotNil(t, board.ID) + require.Equal(t, title, board.Title) + require.Equal(t, model.BoardTypeOpen, board.Type) + require.Equal(t, teamID, board.TeamID) + require.Equal(t, me.ID, board.CreatedBy) + require.Equal(t, me.ID, board.ModifiedBy) + + // move board to custom category + resp = th.Client.UpdateCategoryBoard(teamID, createdCategory.ID, board.ID) + th.CheckOK(resp) + require.NoError(t, resp.Error) + + newBlocks := []model.Block{ + { + ID: utils.NewID(utils.IDTypeBlock), + BoardID: board.ID, + CreateAt: 1, + UpdateAt: 1, + Title: "View 1", + Type: model.TypeView, + }, + } + + newBlocks, resp = th.Client.InsertBlocks(board.ID, newBlocks) + require.NoError(t, resp.Error) + require.Len(t, newBlocks, 1) + + newUserMember := &model.BoardMember{ + UserID: th.GetUser2().ID, + BoardID: board.ID, + SchemeEditor: true, + } + th.Client.AddMemberToBoard(newUserMember) + + members, err := th.Server.App().GetMembersForBoard(board.ID) + require.NoError(t, err) + require.Len(t, members, 2) + + // Duplicate the board + rBoardsAndBlock, resp := th.Client.DuplicateBoard(board.ID, false, teamID) + th.CheckOK(resp) + require.NotNil(t, rBoardsAndBlock) + require.Equal(t, len(rBoardsAndBlock.Boards), 1) + require.Equal(t, len(rBoardsAndBlock.Blocks), 1) + + duplicateBoard := rBoardsAndBlock.Boards[0] + require.Equal(t, duplicateBoard.Type, model.BoardTypePrivate, "Duplicated board should be private") + require.Equal(t, "Public board copy", duplicateBoard.Title) + + members, err = th.Server.App().GetMembersForBoard(duplicateBoard.ID) + require.NoError(t, err) + require.Len(t, members, 1, "Duplicated board should only have one member") + require.Equal(t, me.ID, members[0].UserID) + require.Equal(t, duplicateBoard.ID, members[0].BoardID) + require.True(t, members[0].SchemeAdmin) + + // verify duplicated board is in the same custom category + userCategoryBoards, resp := th.Client.GetUserCategoryBoards(teamID) + th.CheckOK(resp) + require.NotNil(t, rBoardsAndBlock) + + var duplicateBoardCategoryID string + for _, categoryBoard := range userCategoryBoards { + for _, boardID := range categoryBoard.BoardIDs { + if boardID == duplicateBoard.ID { + duplicateBoardCategoryID = categoryBoard.Category.ID + } + } + } + require.Equal(t, createdCategory.ID, duplicateBoardCategoryID) + }) } func TestJoinBoard(t *testing.T) { diff --git a/server/model/category.go b/server/model/category.go index 26caa40f7..4ec582638 100644 --- a/server/model/category.go +++ b/server/model/category.go @@ -1,6 +1,8 @@ package model import ( + "encoding/json" + "io" "strings" "github.com/mattermost/focalboard/server/utils" @@ -81,3 +83,9 @@ func newErrInvalidCategory(msg string) *ErrInvalidCategory { func (e *ErrInvalidCategory) Error() string { return e.msg } + +func CategoryFromJSON(data io.Reader) *Category { + var category *Category + _ = json.NewDecoder(data).Decode(&category) + return category +} diff --git a/webapp/src/components/sidebar/sidebarBoardItem.tsx b/webapp/src/components/sidebar/sidebarBoardItem.tsx index 7f6fa8058..254740c14 100644 --- a/webapp/src/components/sidebar/sidebarBoardItem.tsx +++ b/webapp/src/components/sidebar/sidebarBoardItem.tsx @@ -15,7 +15,7 @@ import MenuWrapper from '../../widgets/menuWrapper' import BoardPermissionGate from '../permissions/boardPermissionGate' import './sidebarBoardItem.scss' -import {CategoryBoards} from '../../store/sidebar' +import {CategoryBoards, updateBoardCategories} from '../../store/sidebar' import CreateNewFolder from '../../widgets/icons/newFolder' import {useAppDispatch, useAppSelector} from '../../store/hooks' import {getCurrentBoardViews, getCurrentViewId} from '../../store/views' @@ -113,6 +113,26 @@ const SidebarBoardItem = (props: Props) => { } const boardId = blocksAndBoards.boards[0].id + + // If the source board is in a custom category, set the new board in + // the same category. Even though the server does this as well on its side, + // we need to do this to avoid the duplicated board showing up in default "Boards" category first + // then jumping to the custom category. + // The jump would happen because when server clones a board from a custom category, + // two WS events are sent - first to indicate the new board belongs to the specific category, + // second, to indicate the new board is created. Depending on the order of execution of the two + // WS event handlers, if the handler for second events executes first, it will show the new board + // in default category in LHS, then when the handler for first events gets executed, it moves the board + // to the correct category. + // By not waiting for the board-category WS event and setting the right category for the board, + // we avoid the jumping behavior. + if (props.categoryBoards.id !== '') { + await dispatch(updateBoardCategories([{ + boardID: boardId, + categoryID: props.categoryBoards.id, + }])) + } + Utils.showBoard(boardId, match, history) }, [board.id])