From 8d17dd820ee18717f71629f1240c5c3c81b4978c Mon Sep 17 00:00:00 2001 From: Harshil Sharma <18575143+harshilsharma63@users.noreply.github.com> Date: Wed, 26 Oct 2022 16:38:03 +0530 Subject: [PATCH] Boards as persisted category (#3877) * WIP * WIP * Removed unused webapp util * Added server tests * Lint fix * Updating existing tests * Updating existing tests * Updating existing tests * Fixing existing tests * Fixing existing tests * Fixing existing tests * Added category type and tests * updated tests * Fixed integration test * type fix * removed seconds from boards name * wip * debugging cy test * Fixed a bug preventing users from collapsing boards category * Debugging cypress test * CI * debugging cy test * Testing a fix * reverting test fix * Handled personal server * Fixed a case for personal server * fixed a test --- server/api/api.go | 1 + server/api/boards.go | 4 + server/app/boards.go | 49 ++- server/app/boards_and_blocks.go | 8 + server/app/boards_test.go | 47 +++ server/app/category.go | 24 ++ server/app/category_boards.go | 100 +++++- server/app/category_boards_test.go | 82 +++++ server/app/category_test.go | 295 ++++++++++++++++++ server/app/import_test.go | 10 + server/app/onboarding_test.go | 43 ++- .../boards_and_blocks_test.go | 6 +- server/integrationtests/permissions_test.go | 46 ++- server/model/category.go | 17 + server/services/store/sqlstore/category.go | 19 +- .../000029_add_category_type_field.down.sql | 1 + .../000029_add_category_type_field.up.sql | 2 + server/utils/testUtils.go | 5 + webapp/cypress/integration/createBoard.ts | 3 + .../__snapshots__/workspace.test.tsx.snap | 84 ----- .../__snapshots__/sidebar.test.tsx.snap | 129 -------- .../src/components/sidebar/sidebar.test.tsx | 2 +- webapp/src/components/sidebar/sidebar.tsx | 12 +- .../components/sidebar/sidebarCategory.tsx | 22 +- webapp/src/components/sidebar/utils.ts | 35 --- webapp/src/store/sidebar.ts | 3 + webapp/src/test/testBlockFactory.ts | 1 + 27 files changed, 746 insertions(+), 304 deletions(-) create mode 100644 server/app/category_boards_test.go create mode 100644 server/app/category_test.go create mode 100644 server/services/store/sqlstore/migrations/000029_add_category_type_field.down.sql create mode 100644 server/services/store/sqlstore/migrations/000029_add_category_type_field.up.sql create mode 100644 server/utils/testUtils.go delete mode 100644 webapp/src/components/sidebar/utils.ts diff --git a/server/api/api.go b/server/api/api.go index b89d52967..48af006f1 100644 --- a/server/api/api.go +++ b/server/api/api.go @@ -178,6 +178,7 @@ func (a *API) userIsGuest(userID string) (bool, error) { // Response helpers func (a *API) errorResponse(w http.ResponseWriter, r *http.Request, err error) { + a.logger.Error(err.Error()) errorResponse := model.ErrorResponse{Error: err.Error()} switch { diff --git a/server/api/boards.go b/server/api/boards.go index 92f26df68..af4e7cb71 100644 --- a/server/api/boards.go +++ b/server/api/boards.go @@ -489,6 +489,10 @@ func (a *API) handleDuplicateBoard(w http.ResponseWriter, r *http.Request) { return } + if toTeam == "" { + toTeam = board.TeamID + } + if toTeam == "" && !a.permissions.HasPermissionToTeam(userID, board.TeamID, model.PermissionViewTeam) { a.errorResponse(w, r, model.NewErrPermission("access denied to team")) return diff --git a/server/app/boards.go b/server/app/boards.go index 5868d3b6e..e7e40f87f 100644 --- a/server/app/boards.go +++ b/server/app/boards.go @@ -21,6 +21,8 @@ var ( const linkBoardMessage = "@%s linked the board [%s](%s) with this channel" const unlinkBoardMessage = "@%s unlinked the board [%s](%s) with this channel" +var errNoDefaultCategoryFound = errors.New("no default category found for user") + func (a *App) GetBoard(boardID string) (*model.Board, error) { board, err := a.store.GetBoard(boardID) if err != nil { @@ -142,7 +144,7 @@ func (a *App) getBoardDescendantModifiedInfo(boardID string, latest bool) (int64 return timestamp, modifiedBy, nil } -func (a *App) setBoardCategoryFromSource(sourceBoardID, destinationBoardID, userID, teamID string) error { +func (a *App) setBoardCategoryFromSource(sourceBoardID, destinationBoardID, userID, teamID string, asTemplate bool) error { // find source board's category ID for the user userCategoryBoards, err := a.GetUserCategoryBoards(userID, teamID) if err != nil { @@ -161,10 +163,14 @@ func (a *App) setBoardCategoryFromSource(sourceBoardID, destinationBoardID, user } } - // if source board is not mapped to a category for this user, - // then we have nothing more to do. if destinationCategoryID == "" { - return nil + // if source board is not mapped to a category for this user, + // then move new board to default category + if !asTemplate { + return a.addBoardsToDefaultCategory(userID, teamID, []*model.Board{{ID: destinationBoardID}}) + } else { + return nil + } } // now that we have source board's category, @@ -184,7 +190,7 @@ func (a *App) DuplicateBoard(boardID, userID, toTeam string, asTemplate bool) (* } for _, board := range bab.Boards { - if categoryErr := a.setBoardCategoryFromSource(boardID, board.ID, userID, board.TeamID); categoryErr != nil { + if categoryErr := a.setBoardCategoryFromSource(boardID, board.ID, userID, toTeam, asTemplate); categoryErr != nil { return nil, nil, categoryErr } } @@ -294,9 +300,42 @@ func (a *App) CreateBoard(board *model.Board, userID string, addMember bool) (*m return nil }) + if board.TeamID != "0" { + if err := a.addBoardsToDefaultCategory(userID, newBoard.TeamID, []*model.Board{newBoard}); err != nil { + return nil, err + } + } + return newBoard, nil } +func (a *App) addBoardsToDefaultCategory(userID, teamID string, boards []*model.Board) error { + userCategoryBoards, err := a.GetUserCategoryBoards(userID, teamID) + if err != nil { + return err + } + + defaultCategoryID := "" + for _, categoryBoard := range userCategoryBoards { + if categoryBoard.Name == defaultCategoryBoards { + defaultCategoryID = categoryBoard.ID + break + } + } + + if defaultCategoryID == "" { + return fmt.Errorf("%w userID: %s", errNoDefaultCategoryFound, userID) + } + + for _, board := range boards { + if err := a.AddUpdateUserCategoryBoard(teamID, userID, defaultCategoryID, board.ID); err != nil { + return err + } + } + + return nil +} + func (a *App) PatchBoard(patch *model.BoardPatch, boardID, userID string) (*model.Board, error) { var oldChannelID string var isTemplate bool diff --git a/server/app/boards_and_blocks.go b/server/app/boards_and_blocks.go index 93146f834..2dc365638 100644 --- a/server/app/boards_and_blocks.go +++ b/server/app/boards_and_blocks.go @@ -55,6 +55,14 @@ func (a *App) CreateBoardsAndBlocks(bab *model.BoardsAndBlocks, userID string, a }() } + for _, board := range newBab.Boards { + if !board.IsTemplate { + if err := a.addBoardsToDefaultCategory(userID, board.TeamID, []*model.Board{board}); err != nil { + return nil, err + } + } + } + return newBab, nil } diff --git a/server/app/boards_test.go b/server/app/boards_test.go index eafb0f0ec..83167bf33 100644 --- a/server/app/boards_test.go +++ b/server/app/boards_test.go @@ -3,6 +3,10 @@ package app import ( "testing" + "github.com/mattermost/focalboard/server/utils" + + "github.com/stretchr/testify/assert" + "github.com/mattermost/focalboard/server/model" "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" @@ -378,3 +382,46 @@ func TestGetBoardCount(t *testing.T) { require.Equal(t, boardCount, count) }) } + +func TestBoardCategory(t *testing.T) { + th, tearDown := SetupTestHelper(t) + defer tearDown() + + t.Run("test addBoardsToDefaultCategory", func(t *testing.T) { + t.Run("no boards default category exists", func(t *testing.T) { + th.Store.EXPECT().GetUserCategoryBoards("user_id", "team_id").Return([]model.CategoryBoards{ + { + Category: model.Category{ID: "category_id_1", Name: "Category 1"}, + BoardIDs: []string{"board_id_1", "board_id_2"}, + }, + { + Category: model.Category{ID: "category_id_2", Name: "Category 2"}, + BoardIDs: []string{"board_id_3"}, + }, + { + Category: model.Category{ID: "category_id_3", Name: "Category 3"}, + BoardIDs: []string{}, + }, + }, nil) + + th.Store.EXPECT().CreateCategory(utils.Anything).Return(nil) + th.Store.EXPECT().GetCategory(utils.Anything).Return(&model.Category{ + ID: "default_category_id", + Name: "Boards", + }, nil) + th.Store.EXPECT().GetBoardsForUserAndTeam("user_id", "team_id", false).Return([]*model.Board{}, nil) + th.Store.EXPECT().AddUpdateCategoryBoard("user_id", "default_category_id", "board_id_1").Return(nil) + th.Store.EXPECT().AddUpdateCategoryBoard("user_id", "default_category_id", "board_id_2").Return(nil) + th.Store.EXPECT().AddUpdateCategoryBoard("user_id", "default_category_id", "board_id_3").Return(nil) + + boards := []*model.Board{ + {ID: "board_id_1"}, + {ID: "board_id_2"}, + {ID: "board_id_3"}, + } + + err := th.App.addBoardsToDefaultCategory("user_id", "team_id", boards) + assert.NoError(t, err) + }) + }) +} diff --git a/server/app/category.go b/server/app/category.go index cd4589894..238988795 100644 --- a/server/app/category.go +++ b/server/app/category.go @@ -1,10 +1,15 @@ package app import ( + "errors" + "github.com/mattermost/focalboard/server/model" "github.com/mattermost/focalboard/server/utils" ) +var ErrCannotDeleteSystemCategory = errors.New("cannot delete a system category") +var ErrCannotUpdateSystemCategory = errors.New("cannot update a system category") + func (a *App) CreateCategory(category *model.Category) (*model.Category, error) { category.Hydrate() if err := category.IsValid(); err != nil { @@ -28,6 +33,10 @@ func (a *App) CreateCategory(category *model.Category) (*model.Category, error) } func (a *App) UpdateCategory(category *model.Category) (*model.Category, error) { + if err := category.IsValid(); err != nil { + return nil, err + } + // verify if category belongs to the user existingCategory, err := a.store.GetCategory(category.ID) if err != nil { @@ -42,6 +51,17 @@ func (a *App) UpdateCategory(category *model.Category) (*model.Category, error) return nil, model.ErrCategoryPermissionDenied } + if existingCategory.TeamID != category.TeamID { + return nil, model.ErrCategoryPermissionDenied + } + + if existingCategory.Type == model.CategoryTypeSystem { + // You cannot rename or delete a system category, + // So restoring its name and undeleting it if set so. + category.Name = existingCategory.Name + category.DeleteAt = 0 + } + category.UpdateAt = utils.GetMillis() if err = category.IsValid(); err != nil { return nil, err @@ -84,6 +104,10 @@ func (a *App) DeleteCategory(categoryID, userID, teamID string) (*model.Category return nil, model.NewErrInvalidCategory("category doesn't belong to the team") } + if existingCategory.Type == model.CategoryTypeSystem { + return nil, ErrCannotDeleteSystemCategory + } + if err = a.store.DeleteCategory(categoryID, userID, teamID); err != nil { return nil, err } diff --git a/server/app/category_boards.go b/server/app/category_boards.go index 61df71944..582cdba4f 100644 --- a/server/app/category_boards.go +++ b/server/app/category_boards.go @@ -1,9 +1,105 @@ package app -import "github.com/mattermost/focalboard/server/model" +import ( + "fmt" + + "github.com/mattermost/focalboard/server/model" +) + +const defaultCategoryBoards = "Boards" func (a *App) GetUserCategoryBoards(userID, teamID string) ([]model.CategoryBoards, error) { - return a.store.GetUserCategoryBoards(userID, teamID) + categoryBoards, err := a.store.GetUserCategoryBoards(userID, teamID) + if err != nil { + return nil, err + } + + createdCategoryBoards, err := a.createDefaultCategoriesIfRequired(categoryBoards, userID, teamID) + if err != nil { + return nil, err + } + + categoryBoards = append(categoryBoards, createdCategoryBoards...) + return categoryBoards, nil +} + +func (a *App) createDefaultCategoriesIfRequired(existingCategoryBoards []model.CategoryBoards, userID, teamID string) ([]model.CategoryBoards, error) { + createdCategories := []model.CategoryBoards{} + + boardsCategoryExist := false + for _, categoryBoard := range existingCategoryBoards { + if categoryBoard.Name == defaultCategoryBoards { + boardsCategoryExist = true + } + } + + if !boardsCategoryExist { + createdCategoryBoards, err := a.createBoardsCategory(userID, teamID, existingCategoryBoards) + if err != nil { + return nil, err + } + + createdCategories = append(createdCategories, *createdCategoryBoards) + } + + return createdCategories, nil +} + +func (a *App) createBoardsCategory(userID, teamID string, existingCategoryBoards []model.CategoryBoards) (*model.CategoryBoards, error) { + // create the category + category := model.Category{ + Name: defaultCategoryBoards, + UserID: userID, + TeamID: teamID, + Collapsed: false, + Type: model.CategoryTypeSystem, + } + createdCategory, err := a.CreateCategory(&category) + if err != nil { + return nil, fmt.Errorf("createBoardsCategory default category creation failed: %w", err) + } + + // once the category is created, we need to move all boards which do not + // belong to any category, into this category. + + userBoards, err := a.GetBoardsForUserAndTeam(userID, teamID, false) + if err != nil { + return nil, fmt.Errorf("createBoardsCategory error fetching user's team's boards: %w", err) + } + + createdCategoryBoards := &model.CategoryBoards{ + Category: *createdCategory, + BoardIDs: []string{}, + } + + for _, board := range userBoards { + belongsToCategory := false + + for _, categoryBoard := range existingCategoryBoards { + for _, boardID := range categoryBoard.BoardIDs { + if boardID == board.ID { + belongsToCategory = true + break + } + } + + // stop looking into other categories if + // the board was found in a category + if belongsToCategory { + break + } + } + + if !belongsToCategory { + if err := a.AddUpdateUserCategoryBoard(teamID, userID, createdCategory.ID, board.ID); err != nil { + return nil, fmt.Errorf("createBoardsCategory failed to add category-less board to the default category, defaultCategoryID: %s, error: %w", createdCategory.ID, err) + } + + createdCategoryBoards.BoardIDs = append(createdCategoryBoards.BoardIDs, board.ID) + } + } + + return createdCategoryBoards, nil } func (a *App) AddUpdateUserCategoryBoard(teamID, userID, categoryID, boardID string) error { diff --git a/server/app/category_boards_test.go b/server/app/category_boards_test.go new file mode 100644 index 000000000..633ef6499 --- /dev/null +++ b/server/app/category_boards_test.go @@ -0,0 +1,82 @@ +package app + +import ( + "testing" + + "github.com/mattermost/focalboard/server/utils" + + "github.com/mattermost/focalboard/server/model" + "github.com/stretchr/testify/assert" +) + +func TestGetUserCategoryBoards(t *testing.T) { + th, tearDown := SetupTestHelper(t) + defer tearDown() + + t.Run("user had no default category and had boards", func(t *testing.T) { + th.Store.EXPECT().GetUserCategoryBoards("user_id", "team_id").Return([]model.CategoryBoards{}, nil) + th.Store.EXPECT().CreateCategory(utils.Anything).Return(nil) + th.Store.EXPECT().GetCategory(utils.Anything).Return(&model.Category{ + ID: "boards_category_id", + Name: "Boards", + }, nil) + + board1 := &model.Board{ + ID: "board_id_1", + } + + board2 := &model.Board{ + ID: "board_id_2", + } + + board3 := &model.Board{ + ID: "board_id_3", + } + + th.Store.EXPECT().GetBoardsForUserAndTeam("user_id", "team_id", false).Return([]*model.Board{board1, board2, board3}, nil) + th.Store.EXPECT().AddUpdateCategoryBoard("user_id", "boards_category_id", "board_id_1").Return(nil) + th.Store.EXPECT().AddUpdateCategoryBoard("user_id", "boards_category_id", "board_id_2").Return(nil) + th.Store.EXPECT().AddUpdateCategoryBoard("user_id", "boards_category_id", "board_id_3").Return(nil) + + categoryBoards, err := th.App.GetUserCategoryBoards("user_id", "team_id") + assert.NoError(t, err) + assert.Equal(t, 1, len(categoryBoards)) + assert.Equal(t, "Boards", categoryBoards[0].Name) + assert.Equal(t, 3, len(categoryBoards[0].BoardIDs)) + assert.Contains(t, categoryBoards[0].BoardIDs, "board_id_1") + assert.Contains(t, categoryBoards[0].BoardIDs, "board_id_2") + assert.Contains(t, categoryBoards[0].BoardIDs, "board_id_3") + }) + + t.Run("user had no default category BUT had no boards", func(t *testing.T) { + th.Store.EXPECT().GetUserCategoryBoards("user_id", "team_id").Return([]model.CategoryBoards{}, nil) + th.Store.EXPECT().CreateCategory(utils.Anything).Return(nil) + th.Store.EXPECT().GetCategory(utils.Anything).Return(&model.Category{ + ID: "boards_category_id", + Name: "Boards", + }, nil) + + th.Store.EXPECT().GetBoardsForUserAndTeam("user_id", "team_id", false).Return([]*model.Board{}, nil) + + categoryBoards, err := th.App.GetUserCategoryBoards("user_id", "team_id") + assert.NoError(t, err) + assert.Equal(t, 1, len(categoryBoards)) + assert.Equal(t, "Boards", categoryBoards[0].Name) + assert.Equal(t, 0, len(categoryBoards[0].BoardIDs)) + }) + + t.Run("user already had a default Boards category with boards in it", func(t *testing.T) { + th.Store.EXPECT().GetUserCategoryBoards("user_id", "team_id").Return([]model.CategoryBoards{ + { + Category: model.Category{Name: "Boards"}, + BoardIDs: []string{"board_id_1", "board_id_2"}, + }, + }, nil) + + categoryBoards, err := th.App.GetUserCategoryBoards("user_id", "team_id") + assert.NoError(t, err) + assert.Equal(t, 1, len(categoryBoards)) + assert.Equal(t, "Boards", categoryBoards[0].Name) + assert.Equal(t, 2, len(categoryBoards[0].BoardIDs)) + }) +} diff --git a/server/app/category_test.go b/server/app/category_test.go new file mode 100644 index 000000000..191b18363 --- /dev/null +++ b/server/app/category_test.go @@ -0,0 +1,295 @@ +package app + +import ( + "testing" + + "github.com/mattermost/focalboard/server/model" + "github.com/mattermost/focalboard/server/utils" + "github.com/stretchr/testify/assert" +) + +func TestCreateCategory(t *testing.T) { + th, tearDown := SetupTestHelper(t) + defer tearDown() + + t.Run("base case", func(t *testing.T) { + th.Store.EXPECT().CreateCategory(utils.Anything).Return(nil) + + th.Store.EXPECT().GetCategory(utils.Anything).Return(&model.Category{ + ID: "category_id_1", + }, nil) + + category := &model.Category{ + Name: "Category", + UserID: "user_id", + TeamID: "team_id", + Type: "custom", + } + createdCategory, err := th.App.CreateCategory(category) + assert.NotNil(t, createdCategory) + assert.NoError(t, err) + }) + + t.Run("creating invalid category", func(t *testing.T) { + category := &model.Category{ + Name: "", // empty name shouldn't be allowed + UserID: "user_id", + TeamID: "team_id", + Type: "custom", + } + createdCategory, err := th.App.CreateCategory(category) + assert.Nil(t, createdCategory) + assert.Error(t, err) + + category.Name = "Name" + category.UserID = "" // empty creator user id shouldn't be allowed + createdCategory, err = th.App.CreateCategory(category) + assert.Nil(t, createdCategory) + assert.Error(t, err) + + category.UserID = "user_id" + category.TeamID = "" // empty TeamID shouldn't be allowed + createdCategory, err = th.App.CreateCategory(category) + assert.Nil(t, createdCategory) + assert.Error(t, err) + + category.Type = "invalid" // unknown type shouldn't be allowed + createdCategory, err = th.App.CreateCategory(category) + assert.Nil(t, createdCategory) + assert.Error(t, err) + }) +} + +func TestUpdateCategory(t *testing.T) { + th, tearDown := SetupTestHelper(t) + defer tearDown() + + t.Run("base case", func(t *testing.T) { + th.Store.EXPECT().GetCategory(utils.Anything).Return(&model.Category{ + ID: "category_id_1", + Name: "Category", + TeamID: "team_id_1", + UserID: "user_id_1", + Type: "custom", + }, nil) + + th.Store.EXPECT().UpdateCategory(utils.Anything).Return(nil) + th.Store.EXPECT().GetCategory("category_id_1").Return(&model.Category{ + ID: "category_id_1", + Name: "Category", + }, nil) + + category := &model.Category{ + ID: "category_id_1", + Name: "Category", + UserID: "user_id_1", + TeamID: "team_id_1", + Type: "custom", + } + updatedCategory, err := th.App.UpdateCategory(category) + assert.NotNil(t, updatedCategory) + assert.NoError(t, err) + }) + + t.Run("updating invalid category", func(t *testing.T) { + category := &model.Category{ + ID: "category_id_1", + Name: "Name", + UserID: "user_id", + TeamID: "team_id", + Type: "custom", + } + + category.ID = "" + createdCategory, err := th.App.UpdateCategory(category) + assert.Nil(t, createdCategory) + assert.Error(t, err) + + category.ID = "category_id_1" + category.Name = "" + createdCategory, err = th.App.UpdateCategory(category) + assert.Nil(t, createdCategory) + assert.Error(t, err) + + category.Name = "Name" + category.UserID = "" // empty creator user id shouldn't be allowed + createdCategory, err = th.App.UpdateCategory(category) + assert.Nil(t, createdCategory) + assert.Error(t, err) + + category.UserID = "user_id" + category.TeamID = "" // empty TeamID shouldn't be allowed + createdCategory, err = th.App.UpdateCategory(category) + assert.Nil(t, createdCategory) + assert.Error(t, err) + + category.Type = "invalid" // unknown type shouldn't be allowed + createdCategory, err = th.App.UpdateCategory(category) + assert.Nil(t, createdCategory) + assert.Error(t, err) + }) + + t.Run("trying to update someone else's category", func(t *testing.T) { + th.Store.EXPECT().GetCategory(utils.Anything).Return(&model.Category{ + ID: "category_id_1", + Name: "Category", + TeamID: "team_id_1", + UserID: "user_id_1", + Type: "custom", + }, nil) + + category := &model.Category{ + ID: "category_id_1", + Name: "Category", + UserID: "user_id_2", + TeamID: "team_id_1", + Type: "custom", + } + updatedCategory, err := th.App.UpdateCategory(category) + assert.Nil(t, updatedCategory) + assert.Error(t, err) + }) + + t.Run("trying to update some other team's category", func(t *testing.T) { + th.Store.EXPECT().GetCategory(utils.Anything).Return(&model.Category{ + ID: "category_id_1", + Name: "Category", + TeamID: "team_id_1", + UserID: "user_id_1", + Type: "custom", + }, nil) + + category := &model.Category{ + ID: "category_id_1", + Name: "Category", + UserID: "user_id_1", + TeamID: "team_id_2", + Type: "custom", + } + updatedCategory, err := th.App.UpdateCategory(category) + assert.Nil(t, updatedCategory) + assert.Error(t, err) + }) + + t.Run("should not be allowed to rename system category", func(t *testing.T) { + th.Store.EXPECT().GetCategory(utils.Anything).Return(&model.Category{ + ID: "category_id_1", + Name: "Category", + TeamID: "team_id_1", + UserID: "user_id_1", + Type: "system", + }, nil).Times(1) + + th.Store.EXPECT().UpdateCategory(utils.Anything).Return(nil) + + th.Store.EXPECT().GetCategory(utils.Anything).Return(&model.Category{ + ID: "category_id_1", + Name: "Category", + TeamID: "team_id_1", + UserID: "user_id_1", + Type: "system", + Collapsed: true, + }, nil).Times(1) + + category := &model.Category{ + ID: "category_id_1", + Name: "Updated Name", + UserID: "user_id_1", + TeamID: "team_id_1", + Type: "system", + } + updatedCategory, err := th.App.UpdateCategory(category) + assert.NotNil(t, updatedCategory) + assert.NoError(t, err) + assert.Equal(t, "Category", updatedCategory.Name) + }) + + t.Run("should be allowed to collapse and expand any category type", func(t *testing.T) { + th.Store.EXPECT().GetCategory(utils.Anything).Return(&model.Category{ + ID: "category_id_1", + Name: "Category", + TeamID: "team_id_1", + UserID: "user_id_1", + Type: "system", + Collapsed: false, + }, nil).Times(1) + + th.Store.EXPECT().UpdateCategory(utils.Anything).Return(nil) + + th.Store.EXPECT().GetCategory(utils.Anything).Return(&model.Category{ + ID: "category_id_1", + Name: "Category", + TeamID: "team_id_1", + UserID: "user_id_1", + Type: "system", + Collapsed: true, + }, nil).Times(1) + + category := &model.Category{ + ID: "category_id_1", + Name: "Updated Name", + UserID: "user_id_1", + TeamID: "team_id_1", + Type: "system", + Collapsed: true, + } + updatedCategory, err := th.App.UpdateCategory(category) + assert.NotNil(t, updatedCategory) + assert.NoError(t, err) + assert.Equal(t, "Category", updatedCategory.Name, "The name should have not been updated") + assert.True(t, updatedCategory.Collapsed) + }) +} + +func TestDeleteCategory(t *testing.T) { + th, tearDown := SetupTestHelper(t) + defer tearDown() + + t.Run("base case", func(t *testing.T) { + th.Store.EXPECT().GetCategory("category_id_1").Return(&model.Category{ + ID: "category_id_1", + DeleteAt: 0, + UserID: "user_id_1", + TeamID: "team_id_1", + Type: "custom", + }, nil) + + th.Store.EXPECT().DeleteCategory("category_id_1", "user_id_1", "team_id_1").Return(nil) + + th.Store.EXPECT().GetCategory("category_id_1").Return(&model.Category{ + DeleteAt: 10000, + }, nil) + + deletedCategory, err := th.App.DeleteCategory("category_id_1", "user_id_1", "team_id_1") + assert.NotNil(t, deletedCategory) + assert.NoError(t, err) + }) + + t.Run("trying to delete already deleted category", func(t *testing.T) { + th.Store.EXPECT().GetCategory("category_id_1").Return(&model.Category{ + ID: "category_id_1", + DeleteAt: 1000, + UserID: "user_id_1", + TeamID: "team_id_1", + Type: "custom", + }, nil) + + deletedCategory, err := th.App.DeleteCategory("category_id_1", "user_id_1", "team_id_1") + assert.NotNil(t, deletedCategory) + assert.NoError(t, err) + }) + + t.Run("trying to delete system category", func(t *testing.T) { + th.Store.EXPECT().GetCategory("category_id_1").Return(&model.Category{ + ID: "category_id_1", + DeleteAt: 0, + UserID: "user_id_1", + TeamID: "team_id_1", + Type: "system", + }, nil) + + deletedCategory, err := th.App.DeleteCategory("category_id_1", "user_id_1", "team_id_1") + assert.Nil(t, deletedCategory) + assert.Error(t, err) + }) +} diff --git a/server/app/import_test.go b/server/app/import_test.go index 61d44b940..ab37e28a8 100644 --- a/server/app/import_test.go +++ b/server/app/import_test.go @@ -4,6 +4,8 @@ import ( "bytes" "testing" + "github.com/mattermost/focalboard/server/utils" + "github.com/golang/mock/gomock" "github.com/mattermost/focalboard/server/model" "github.com/stretchr/testify/require" @@ -47,6 +49,14 @@ func TestApp_ImportArchive(t *testing.T) { th.Store.EXPECT().GetMembersForBoard(board.ID).AnyTimes().Return([]*model.BoardMember{boardMember}, nil) th.Store.EXPECT().GetBoard(board.ID).Return(board, nil) th.Store.EXPECT().GetMemberForBoard(board.ID, "user").Return(boardMember, nil) + th.Store.EXPECT().GetUserCategoryBoards("user", "test-team") + th.Store.EXPECT().CreateCategory(utils.Anything).Return(nil) + th.Store.EXPECT().GetCategory(utils.Anything).Return(&model.Category{ + ID: "boards_category_id", + Name: "Boards", + }, nil) + th.Store.EXPECT().GetBoardsForUserAndTeam("user", "test-team", false).Return([]*model.Board{}, nil) + th.Store.EXPECT().AddUpdateCategoryBoard("user", "boards_category_id", utils.Anything).Return(nil) err := th.App.ImportArchive(r, opts) require.NoError(t, err, "import archive should not fail") diff --git a/server/app/onboarding_test.go b/server/app/onboarding_test.go index 26787413a..5b5fe196b 100644 --- a/server/app/onboarding_test.go +++ b/server/app/onboarding_test.go @@ -3,6 +3,8 @@ package app import ( "testing" + "github.com/mattermost/focalboard/server/utils" + "github.com/mattermost/focalboard/server/model" "github.com/stretchr/testify/assert" ) @@ -26,10 +28,19 @@ func TestPrepareOnboardingTour(t *testing.T) { } th.Store.EXPECT().GetTemplateBoards("0", "").Return([]*model.Board{&welcomeBoard}, nil) - th.Store.EXPECT().DuplicateBoard(welcomeBoard.ID, userID, teamID, false).Return(&model.BoardsAndBlocks{Boards: []*model.Board{&welcomeBoard}}, + th.Store.EXPECT().DuplicateBoard(welcomeBoard.ID, userID, teamID, false).Return(&model.BoardsAndBlocks{Boards: []*model.Board{ + { + ID: "board_id_2", + Title: "Welcome to Boards!", + TeamID: "0", + IsTemplate: true, + }, + }}, nil, nil) - th.Store.EXPECT().GetMembersForBoard(welcomeBoard.ID).Return([]*model.BoardMember{}, nil).Times(3) - th.Store.EXPECT().GetBoard(welcomeBoard.ID).Return(&welcomeBoard, nil).AnyTimes() + th.Store.EXPECT().GetMembersForBoard(welcomeBoard.ID).Return([]*model.BoardMember{}, nil).Times(2) + th.Store.EXPECT().GetMembersForBoard("board_id_2").Return([]*model.BoardMember{}, nil).Times(1) + th.Store.EXPECT().GetBoard(welcomeBoard.ID).Return(&welcomeBoard, nil).Times(1) + th.Store.EXPECT().GetBoard("board_id_2").Return(&welcomeBoard, nil).Times(1) th.Store.EXPECT().GetUsersByTeam("0", "").Return([]*model.User{}, nil) privateWelcomeBoard := model.Board{ @@ -40,7 +51,7 @@ func TestPrepareOnboardingTour(t *testing.T) { Type: model.BoardTypePrivate, } newType := model.BoardTypePrivate - th.Store.EXPECT().PatchBoard("board_id_1", &model.BoardPatch{Type: &newType}, "user_id_1").Return(&privateWelcomeBoard, nil) + th.Store.EXPECT().PatchBoard("board_id_2", &model.BoardPatch{Type: &newType}, "user_id_1").Return(&privateWelcomeBoard, nil) userPreferencesPatch := model.UserPreferencesPatch{ UpdatedFields: map[string]string{ @@ -51,7 +62,22 @@ func TestPrepareOnboardingTour(t *testing.T) { } th.Store.EXPECT().PatchUserPreferences(userID, userPreferencesPatch).Return(nil, nil) - th.Store.EXPECT().GetUserCategoryBoards(userID, "0").Return([]model.CategoryBoards{}, nil) + th.Store.EXPECT().GetUserCategoryBoards(userID, "team_id").Return([]model.CategoryBoards{}, nil).Times(1) + + // when this is called the second time, the default category is created so we need to include that in the response list + th.Store.EXPECT().GetUserCategoryBoards(userID, "team_id").Return([]model.CategoryBoards{ + { + Category: model.Category{ID: "boards_category_id", Name: "Boards"}, + }, + }, nil).Times(1) + + th.Store.EXPECT().CreateCategory(utils.Anything).Return(nil).Times(1) + th.Store.EXPECT().GetCategory(utils.Anything).Return(&model.Category{ + ID: "boards_category", + Name: "Boards", + }, nil) + th.Store.EXPECT().GetBoardsForUserAndTeam("user_id_1", teamID, false).Return([]*model.Board{}, nil) + th.Store.EXPECT().AddUpdateCategoryBoard("user_id_1", "boards_category_id", "board_id_2").Return(nil) teamID, boardID, err := th.App.PrepareOnboardingTour(userID, teamID) assert.NoError(t, err) @@ -89,7 +115,12 @@ 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") + th.Store.EXPECT().GetUserCategoryBoards(userID, "team_id").Return([]model.CategoryBoards{ + { + Category: model.Category{ID: "boards_category_id", Name: "Boards"}, + }, + }, nil).Times(2) + th.Store.EXPECT().AddUpdateCategoryBoard("user_id_1", "boards_category_id", "board_id_1").Return(nil) boardID, err := th.App.createWelcomeBoard(userID, teamID) assert.Nil(t, err) diff --git a/server/integrationtests/boards_and_blocks_test.go b/server/integrationtests/boards_and_blocks_test.go index 78b8442ed..70262e95c 100644 --- a/server/integrationtests/boards_and_blocks_test.go +++ b/server/integrationtests/boards_and_blocks_test.go @@ -729,7 +729,8 @@ func TestDeleteBoardsAndBlocks(t *testing.T) { // the user is an admin of the first board newBoard1 := &model.Board{ - Type: model.BoardTypeOpen, + Type: model.BoardTypeOpen, + TeamID: "team_id_1", } board1, err := th.Server.App().CreateBoard(newBoard1, th.GetUser1().ID, true) require.NoError(t, err) @@ -737,7 +738,8 @@ func TestDeleteBoardsAndBlocks(t *testing.T) { // but not of the second newBoard2 := &model.Board{ - Type: model.BoardTypeOpen, + Type: model.BoardTypeOpen, + TeamID: "team_id_1", } board2, err := th.Server.App().CreateBoard(newBoard2, th.GetUser1().ID, false) require.NoError(t, err) diff --git a/server/integrationtests/permissions_test.go b/server/integrationtests/permissions_test.go index 7122868d6..1187fdad3 100644 --- a/server/integrationtests/permissions_test.go +++ b/server/integrationtests/permissions_test.go @@ -43,6 +43,18 @@ type TestCase struct { totalResults int } +func (tt TestCase) identifier() string { + return fmt.Sprintf( + "url: %s method: %s body: %s userRoles: %s expectedStatusCode: %d totalResults: %d", + tt.url, + tt.method, + tt.body, + tt.userRole, + tt.expectedStatusCode, + tt.totalResults, + ) +} + func setupClients(th *TestHelper) Clients { // user1 clients := Clients{ @@ -262,7 +274,6 @@ func runTestCases(t *testing.T, ttCases []TestCase, testData TestData, clients C url = strings.ReplaceAll(url, "{PUBLIC_BOARD_ID}", testData.publicBoard.ID) url = strings.ReplaceAll(url, "{PUBLIC_TEMPLATE_ID}", testData.publicTemplate.ID) url = strings.ReplaceAll(url, "{PRIVATE_TEMPLATE_ID}", testData.privateTemplate.ID) - url = strings.ReplaceAll(url, "{USER_ANON_ID}", userAnonID) url = strings.ReplaceAll(url, "{USER_NO_TEAM_MEMBER_ID}", userNoTeamMemberID) url = strings.ReplaceAll(url, "{USER_TEAM_MEMBER_ID}", userTeamMemberID) @@ -273,7 +284,7 @@ func runTestCases(t *testing.T, ttCases []TestCase, testData TestData, clients C url = strings.ReplaceAll(url, "{USER_GUEST_ID}", userGuestID) if strings.Contains(url, "{") || strings.Contains(url, "}") { - require.Fail(t, "Unreplaced tokens in url", url) + require.Fail(t, "Unreplaced tokens in url", url, tc.identifier()) } var response *http.Response @@ -296,28 +307,28 @@ func runTestCases(t *testing.T, ttCases []TestCase, testData TestData, clients C defer response.Body.Close() } - require.Equal(t, tc.expectedStatusCode, response.StatusCode) + require.Equal(t, tc.expectedStatusCode, response.StatusCode, tc.identifier()) if tc.expectedStatusCode >= 200 && tc.expectedStatusCode < 300 { - require.NoError(t, err) + require.NoError(t, err, tc.identifier()) } if tc.expectedStatusCode >= 200 && tc.expectedStatusCode < 300 { body, err := io.ReadAll(response.Body) if err != nil { - require.Fail(t, err.Error()) + require.Fail(t, err.Error(), tc.identifier()) } if strings.HasPrefix(string(body), "[") { var data []interface{} err = json.Unmarshal(body, &data) if err != nil { - require.Fail(t, err.Error()) + require.Fail(t, err.Error(), tc.identifier()) } - require.Len(t, data, tc.totalResults) + require.Len(t, data, tc.totalResults, tc.identifier()) } else { if tc.totalResults > 0 { require.Equal(t, 1, tc.totalResults) - require.Greater(t, len(string(body)), 2) + require.Greater(t, len(string(body)), 2, tc.identifier()) } else { - require.Len(t, string(body), 2) + require.Len(t, string(body), 2, tc.identifier()) } } } @@ -2865,14 +2876,14 @@ func TestPermissionsClientConfig(t *testing.T) { func TestPermissionsGetCategories(t *testing.T) { ttCases := []TestCase{ - {"/teams/test-team/categories", methodGet, "", userAnon, http.StatusUnauthorized, 0}, - {"/teams/test-team/categories", methodGet, "", userNoTeamMember, http.StatusOK, 0}, - {"/teams/test-team/categories", methodGet, "", userTeamMember, http.StatusOK, 0}, - {"/teams/test-team/categories", methodGet, "", userViewer, http.StatusOK, 0}, - {"/teams/test-team/categories", methodGet, "", userCommenter, http.StatusOK, 0}, - {"/teams/test-team/categories", methodGet, "", userEditor, http.StatusOK, 0}, - {"/teams/test-team/categories", methodGet, "", userAdmin, http.StatusOK, 0}, - {"/teams/test-team/categories", methodGet, "", userGuest, http.StatusOK, 0}, + {"/teams/test-team/categories", methodGet, "", userAnon, http.StatusUnauthorized, 1}, + {"/teams/test-team/categories", methodGet, "", userNoTeamMember, http.StatusOK, 1}, + {"/teams/test-team/categories", methodGet, "", userTeamMember, http.StatusOK, 1}, + {"/teams/test-team/categories", methodGet, "", userViewer, http.StatusOK, 1}, + {"/teams/test-team/categories", methodGet, "", userCommenter, http.StatusOK, 1}, + {"/teams/test-team/categories", methodGet, "", userEditor, http.StatusOK, 1}, + {"/teams/test-team/categories", methodGet, "", userAdmin, http.StatusOK, 1}, + {"/teams/test-team/categories", methodGet, "", userGuest, http.StatusOK, 1}, } t.Run("plugin", func(t *testing.T) { @@ -2960,6 +2971,7 @@ func TestPermissionsUpdateCategory(t *testing.T) { UserID: userID, CreateAt: model.GetMillis(), UpdateAt: model.GetMillis(), + Type: "custom", }) } diff --git a/server/model/category.go b/server/model/category.go index fe49b1a1b..73c372e13 100644 --- a/server/model/category.go +++ b/server/model/category.go @@ -2,12 +2,18 @@ package model import ( "encoding/json" + "fmt" "io" "strings" "github.com/mattermost/focalboard/server/utils" ) +const ( + CategoryTypeSystem = "system" + CategoryTypeCustom = "custom" +) + // Category is a board category // swagger:model type Category struct { @@ -42,12 +48,19 @@ type Category struct { // Category's state in client side // required: true Collapsed bool `json:"collapsed"` + + // Category's type + // required: true + Type string `json:"type"` } func (c *Category) Hydrate() { c.ID = utils.NewID(utils.IDTypeNone) c.CreateAt = utils.GetMillis() c.UpdateAt = c.CreateAt + if c.Type == "" { + c.Type = CategoryTypeCustom + } } func (c *Category) IsValid() error { @@ -67,6 +80,10 @@ func (c *Category) IsValid() error { return NewErrInvalidCategory("category team id ID cannot be empty") } + if c.Type != CategoryTypeCustom && c.Type != CategoryTypeSystem { + return NewErrInvalidCategory(fmt.Sprintf("category type is invalid. Allowed types: %s and %s", CategoryTypeSystem, CategoryTypeCustom)) + } + return nil } diff --git a/server/services/store/sqlstore/category.go b/server/services/store/sqlstore/category.go index 62560a9c6..23272ae05 100644 --- a/server/services/store/sqlstore/category.go +++ b/server/services/store/sqlstore/category.go @@ -12,7 +12,7 @@ import ( func (s *SQLStore) getCategory(db sq.BaseRunner, id string) (*model.Category, error) { query := s.getQueryBuilder(db). - Select("id", "name", "user_id", "team_id", "create_at", "update_at", "delete_at", "collapsed"). + Select("id", "name", "user_id", "team_id", "create_at", "update_at", "delete_at", "collapsed", "type"). From(s.tablePrefix + "categories"). Where(sq.Eq{"id": id}) @@ -47,6 +47,7 @@ func (s *SQLStore) createCategory(db sq.BaseRunner, category model.Category) err "update_at", "delete_at", "collapsed", + "type", ). Values( category.ID, @@ -57,6 +58,7 @@ func (s *SQLStore) createCategory(db sq.BaseRunner, category model.Category) err category.UpdateAt, category.DeleteAt, category.Collapsed, + category.Type, ) _, err := query.Exec() @@ -73,7 +75,10 @@ func (s *SQLStore) updateCategory(db sq.BaseRunner, category model.Category) err Set("name", category.Name). Set("update_at", category.UpdateAt). Set("collapsed", category.Collapsed). - Where(sq.Eq{"id": category.ID}) + Where(sq.Eq{ + "id": category.ID, + "delete_at": 0, + }) _, err := query.Exec() if err != nil { @@ -88,9 +93,10 @@ func (s *SQLStore) deleteCategory(db sq.BaseRunner, categoryID, userID, teamID s Update(s.tablePrefix+"categories"). Set("delete_at", utils.GetMillis()). Where(sq.Eq{ - "id": categoryID, - "user_id": userID, - "team_id": teamID, + "id": categoryID, + "user_id": userID, + "team_id": teamID, + "delete_at": 0, }) _, err := query.Exec() @@ -109,7 +115,7 @@ func (s *SQLStore) deleteCategory(db sq.BaseRunner, categoryID, userID, teamID s func (s *SQLStore) getUserCategories(db sq.BaseRunner, userID, teamID string) ([]model.Category, error) { query := s.getQueryBuilder(db). - Select("id", "name", "user_id", "team_id", "create_at", "update_at", "delete_at", "collapsed"). + Select("id", "name", "user_id", "team_id", "create_at", "update_at", "delete_at", "collapsed", "type"). From(s.tablePrefix + "categories"). Where(sq.Eq{ "user_id": userID, @@ -140,6 +146,7 @@ func (s *SQLStore) categoriesFromRows(rows *sql.Rows) ([]model.Category, error) &category.UpdateAt, &category.DeleteAt, &category.Collapsed, + &category.Type, ) if err != nil { diff --git a/server/services/store/sqlstore/migrations/000029_add_category_type_field.down.sql b/server/services/store/sqlstore/migrations/000029_add_category_type_field.down.sql new file mode 100644 index 000000000..dc45b39e6 --- /dev/null +++ b/server/services/store/sqlstore/migrations/000029_add_category_type_field.down.sql @@ -0,0 +1 @@ +ALTER TABLE {{.prefix}}categories DROP COLUMN type; diff --git a/server/services/store/sqlstore/migrations/000029_add_category_type_field.up.sql b/server/services/store/sqlstore/migrations/000029_add_category_type_field.up.sql new file mode 100644 index 000000000..676ceea01 --- /dev/null +++ b/server/services/store/sqlstore/migrations/000029_add_category_type_field.up.sql @@ -0,0 +1,2 @@ +ALTER TABLE {{.prefix}}categories ADD COLUMN type varchar(64); +UPDATE {{.prefix}}categories SET type = 'custom' WHERE type IS NULL; diff --git a/server/utils/testUtils.go b/server/utils/testUtils.go new file mode 100644 index 000000000..4879b3908 --- /dev/null +++ b/server/utils/testUtils.go @@ -0,0 +1,5 @@ +package utils + +import "github.com/stretchr/testify/mock" + +var Anything = mock.MatchedBy(func(interface{}) bool { return true }) diff --git a/webapp/cypress/integration/createBoard.ts b/webapp/cypress/integration/createBoard.ts index ab3dd3566..7440fbeb2 100644 --- a/webapp/cypress/integration/createBoard.ts +++ b/webapp/cypress/integration/createBoard.ts @@ -133,6 +133,9 @@ describe('Create and delete board / card', () => { // Delete board cy.log('**Delete board**') + cy.get('.Sidebar .octo-sidebar-list').then((el) => { + cy.log(el.text()) + }) cy.get('.Sidebar .octo-sidebar-list'). contains(boardTitle). parent(). diff --git a/webapp/src/components/__snapshots__/workspace.test.tsx.snap b/webapp/src/components/__snapshots__/workspace.test.tsx.snap index 1aef03022..e06582101 100644 --- a/webapp/src/components/__snapshots__/workspace.test.tsx.snap +++ b/webapp/src/components/__snapshots__/workspace.test.tsx.snap @@ -207,48 +207,6 @@ exports[`src/components/workspace return workspace and showcard 1`] = ` -