From 1c383fb17934ac74138cbc1c85a81eadf51e2496 Mon Sep 17 00:00:00 2001 From: Doug Lauder Date: Mon, 22 Aug 2022 19:06:45 -0400 Subject: [PATCH 1/6] Add `disable_notify` to all block APIs that can trigger notifications. (#3720) * Add disable_notify to all block APIs that can trigger notifications. --- server/api/blocks.go | 34 ++++++++++++++++++++++++++----- server/app/blocks.go | 43 +++++++++++++++++++++++++++++++-------- server/app/blocks_test.go | 10 ++++----- 3 files changed, 68 insertions(+), 19 deletions(-) diff --git a/server/api/blocks.go b/server/api/blocks.go index 162d05787..4aef90d4e 100644 --- a/server/api/blocks.go +++ b/server/api/blocks.go @@ -190,7 +190,7 @@ func (a *API) handlePostBlocks(w http.ResponseWriter, r *http.Request) { // type: string // - name: disable_notify // in: query - // description: Disables notifications (for bulk data inserting) + // description: Disables notifications (for bulk inserting) // required: false // type: bool // - name: Body @@ -289,7 +289,7 @@ func (a *API) handlePostBlocks(w http.ResponseWriter, r *http.Request) { } } - newBlocks, err := a.app.InsertBlocks(blocks, session.UserID, !disableNotify) + newBlocks, err := a.app.InsertBlocksAndNotify(blocks, session.UserID, disableNotify) if err != nil { if errors.Is(err, app.ErrViewsLimitReached) { a.errorResponse(w, r.URL.Path, http.StatusBadRequest, err.Error(), err) @@ -336,6 +336,11 @@ func (a *API) handleDeleteBlock(w http.ResponseWriter, r *http.Request) { // description: ID of block to delete // required: true // type: string + // - name: disable_notify + // in: query + // description: Disables notifications (for bulk deletion) + // required: false + // type: bool // security: // - BearerAuth: [] // responses: @@ -353,6 +358,9 @@ func (a *API) handleDeleteBlock(w http.ResponseWriter, r *http.Request) { boardID := vars["boardID"] blockID := vars["blockID"] + val := r.URL.Query().Get("disable_notify") + disableNotify := val == True + if !a.permissions.HasPermissionToBoard(userID, boardID, model.PermissionManageBoardCards) { a.errorResponse(w, r.URL.Path, http.StatusForbidden, "", PermissionError{"access denied to make board changes"}) return @@ -373,7 +381,7 @@ func (a *API) handleDeleteBlock(w http.ResponseWriter, r *http.Request) { auditRec.AddMeta("boardID", boardID) auditRec.AddMeta("blockID", blockID) - err = a.app.DeleteBlock(blockID, userID) + err = a.app.DeleteBlockAndNotify(blockID, userID, disableNotify) if err != nil { a.errorResponse(w, r.URL.Path, http.StatusInternalServerError, "", err) return @@ -497,6 +505,11 @@ func (a *API) handlePatchBlock(w http.ResponseWriter, r *http.Request) { // description: ID of block to patch // required: true // type: string + // - name: disable_notify + // in: query + // description: Disables notifications (for bulk patching) + // required: false + // type: bool // - name: Body // in: body // description: block patch to apply @@ -520,6 +533,9 @@ func (a *API) handlePatchBlock(w http.ResponseWriter, r *http.Request) { boardID := vars["boardID"] blockID := vars["blockID"] + val := r.URL.Query().Get("disable_notify") + disableNotify := val == True + if !a.permissions.HasPermissionToBoard(userID, boardID, model.PermissionManageBoardCards) { a.errorResponse(w, r.URL.Path, http.StatusForbidden, "", PermissionError{"access denied to make board changes"}) return @@ -553,7 +569,7 @@ func (a *API) handlePatchBlock(w http.ResponseWriter, r *http.Request) { auditRec.AddMeta("boardID", boardID) auditRec.AddMeta("blockID", blockID) - err = a.app.PatchBlock(blockID, patch, userID) + err = a.app.PatchBlockAndNotify(blockID, patch, userID, disableNotify) if errors.Is(err, app.ErrPatchUpdatesLimitedCards) { a.errorResponse(w, r.URL.Path, http.StatusForbidden, "", err) return @@ -583,6 +599,11 @@ func (a *API) handlePatchBlocks(w http.ResponseWriter, r *http.Request) { // description: Workspace ID // required: true // type: string + // - name: disable_notify + // in: query + // description: Disables notifications (for bulk patching) + // required: false + // type: bool // - name: Body // in: body // description: block Ids and block patches to apply @@ -606,6 +627,9 @@ func (a *API) handlePatchBlocks(w http.ResponseWriter, r *http.Request) { vars := mux.Vars(r) teamID := vars["teamID"] + val := r.URL.Query().Get("disable_notify") + disableNotify := val == True + requestBody, err := ioutil.ReadAll(r.Body) if err != nil { a.errorResponse(w, r.URL.Path, http.StatusInternalServerError, "", err) @@ -638,7 +662,7 @@ func (a *API) handlePatchBlocks(w http.ResponseWriter, r *http.Request) { } } - err = a.app.PatchBlocks(teamID, patches, userID) + err = a.app.PatchBlocksAndNotify(teamID, patches, userID, disableNotify) if errors.Is(err, app.ErrPatchUpdatesLimitedCards) { a.errorResponse(w, r.URL.Path, http.StatusForbidden, "", err) return diff --git a/server/app/blocks.go b/server/app/blocks.go index aa67662c2..9c287eb99 100644 --- a/server/app/blocks.go +++ b/server/app/blocks.go @@ -66,6 +66,10 @@ func (a *App) DuplicateBlock(boardID string, blockID string, userID string, asTe } func (a *App) PatchBlock(blockID string, blockPatch *model.BlockPatch, modifiedByID string) error { + return a.PatchBlockAndNotify(blockID, blockPatch, modifiedByID, false) +} + +func (a *App) PatchBlockAndNotify(blockID string, blockPatch *model.BlockPatch, modifiedByID string, disableNotify bool) error { oldBlock, err := a.store.GetBlock(blockID) if err != nil { return err @@ -104,13 +108,19 @@ func (a *App) PatchBlock(blockID string, blockPatch *model.BlockPatch, modifiedB a.webhook.NotifyUpdate(*block) // send notifications - a.notifyBlockChanged(notify.Update, block, oldBlock, modifiedByID) + if !disableNotify { + a.notifyBlockChanged(notify.Update, block, oldBlock, modifiedByID) + } return nil }) return nil } func (a *App) PatchBlocks(teamID string, blockPatches *model.BlockPatchBatch, modifiedByID string) error { + return a.PatchBlocksAndNotify(teamID, blockPatches, modifiedByID, false) +} + +func (a *App) PatchBlocksAndNotify(teamID string, blockPatches *model.BlockPatchBatch, modifiedByID string, disableNotify bool) error { oldBlocks, err := a.store.GetBlocksByIDs(blockPatches.BlockIDs) if err != nil { return err @@ -139,7 +149,9 @@ func (a *App) PatchBlocks(teamID string, blockPatches *model.BlockPatchBatch, mo } a.wsAdapter.BroadcastBlockChange(teamID, *newBlock) a.webhook.NotifyUpdate(*newBlock) - a.notifyBlockChanged(notify.Update, newBlock, &oldBlocks[i], modifiedByID) + if !disableNotify { + a.notifyBlockChanged(notify.Update, newBlock, &oldBlocks[i], modifiedByID) + } } return nil }) @@ -147,6 +159,10 @@ func (a *App) PatchBlocks(teamID string, blockPatches *model.BlockPatchBatch, mo } func (a *App) InsertBlock(block model.Block, modifiedByID string) error { + return a.InsertBlockAndNotify(block, modifiedByID, false) +} + +func (a *App) InsertBlockAndNotify(block model.Block, modifiedByID string, disableNotify bool) error { board, bErr := a.store.GetBoard(block.BoardID) if bErr != nil { return bErr @@ -158,8 +174,9 @@ func (a *App) InsertBlock(block model.Block, modifiedByID string) error { a.wsAdapter.BroadcastBlockChange(board.TeamID, block) a.metrics.IncrementBlocksInserted(1) a.webhook.NotifyUpdate(block) - a.notifyBlockChanged(notify.Add, &block, nil, modifiedByID) - + if !disableNotify { + a.notifyBlockChanged(notify.Add, &block, nil, modifiedByID) + } return nil }) } @@ -198,7 +215,11 @@ func (a *App) isWithinViewsLimit(boardID string, block model.Block) (bool, error return len(views) < limits.Views, nil } -func (a *App) InsertBlocks(blocks []model.Block, modifiedByID string, allowNotifications bool) ([]model.Block, error) { +func (a *App) InsertBlocks(blocks []model.Block, modifiedByID string) ([]model.Block, error) { + return a.InsertBlocksAndNotify(blocks, modifiedByID, false) +} + +func (a *App) InsertBlocksAndNotify(blocks []model.Block, modifiedByID string, disableNotify bool) ([]model.Block, error) { if len(blocks) == 0 { return []model.Block{}, nil } @@ -246,11 +267,10 @@ func (a *App) InsertBlocks(blocks []model.Block, modifiedByID string, allowNotif for _, b := range needsNotify { block := b a.webhook.NotifyUpdate(block) - if allowNotifications { + if !disableNotify { a.notifyBlockChanged(notify.Add, &block, nil, modifiedByID) } } - return nil }) @@ -331,6 +351,10 @@ func (a *App) GetBlockByID(blockID string) (*model.Block, error) { } func (a *App) DeleteBlock(blockID string, modifiedBy string) error { + return a.DeleteBlockAndNotify(blockID, modifiedBy, false) +} + +func (a *App) DeleteBlockAndNotify(blockID string, modifiedBy string, disableNotify bool) error { block, err := a.store.GetBlock(blockID) if err != nil { return err @@ -368,8 +392,9 @@ func (a *App) DeleteBlock(blockID string, modifiedBy string) error { a.blockChangeNotifier.Enqueue(func() error { a.wsAdapter.BroadcastBlockDelete(board.TeamID, blockID, block.BoardID) a.metrics.IncrementBlocksDeleted(1) - a.notifyBlockChanged(notify.Delete, block, block, modifiedBy) - + if !disableNotify { + a.notifyBlockChanged(notify.Delete, block, block, modifiedBy) + } return nil }) diff --git a/server/app/blocks_test.go b/server/app/blocks_test.go index abd75bfd6..f15d66d45 100644 --- a/server/app/blocks_test.go +++ b/server/app/blocks_test.go @@ -287,7 +287,7 @@ func TestInsertBlocks(t *testing.T) { th.Store.EXPECT().GetBoard(boardID).Return(board, nil) th.Store.EXPECT().InsertBlock(&block, "user-id-1").Return(nil) th.Store.EXPECT().GetMembersForBoard(boardID).Return([]*model.BoardMember{}, nil) - _, err := th.App.InsertBlocks([]model.Block{block}, "user-id-1", false) + _, err := th.App.InsertBlocks([]model.Block{block}, "user-id-1") require.NoError(t, err) }) @@ -297,7 +297,7 @@ func TestInsertBlocks(t *testing.T) { board := &model.Board{ID: boardID} th.Store.EXPECT().GetBoard(boardID).Return(board, nil) th.Store.EXPECT().InsertBlock(&block, "user-id-1").Return(blockError{"error"}) - _, err := th.App.InsertBlocks([]model.Block{block}, "user-id-1", false) + _, err := th.App.InsertBlocks([]model.Block{block}, "user-id-1") require.Error(t, err, "error") }) @@ -329,7 +329,7 @@ func TestInsertBlocks(t *testing.T) { th.Store.EXPECT().GetCardLimitTimestamp().Return(int64(1), nil) th.Store.EXPECT().GetBlocksWithParentAndType("test-board-id", "parent_id", "view").Return([]model.Block{{}}, nil) - _, err := th.App.InsertBlocks([]model.Block{block}, "user-id-1", false) + _, err := th.App.InsertBlocks([]model.Block{block}, "user-id-1") require.NoError(t, err) }) @@ -359,7 +359,7 @@ func TestInsertBlocks(t *testing.T) { th.Store.EXPECT().GetCardLimitTimestamp().Return(int64(1), nil) th.Store.EXPECT().GetBlocksWithParentAndType("test-board-id", "parent_id", "view").Return([]model.Block{{}, {}}, nil) - _, err := th.App.InsertBlocks([]model.Block{block}, "user-id-1", false) + _, err := th.App.InsertBlocks([]model.Block{block}, "user-id-1") require.Error(t, err) }) @@ -400,7 +400,7 @@ func TestInsertBlocks(t *testing.T) { th.Store.EXPECT().GetCardLimitTimestamp().Return(int64(1), nil).Times(2) th.Store.EXPECT().GetBlocksWithParentAndType("test-board-id", "parent_id", "view").Return([]model.Block{{}}, nil).Times(2) - _, err := th.App.InsertBlocks([]model.Block{view1, view2}, "user-id-1", false) + _, err := th.App.InsertBlocks([]model.Block{view1, view2}, "user-id-1") require.Error(t, err) }) } From 27a4da126d56e5fd5c28fedc264c1d7be65c3110 Mon Sep 17 00:00:00 2001 From: Rajat Dabade Date: Tue, 23 Aug 2022 17:01:40 +0530 Subject: [PATCH 2/6] Added role so that the non-admin will not be able to unlink the board (#3703) Co-authored-by: Asaad Mahmood --- .../rhsChannelBoardItem.test.tsx.snap | 19 +- .../src/components/rhsChannelBoardItem.scss | 10 +- .../components/rhsChannelBoardItem.test.tsx | 14 +- .../src/components/rhsChannelBoardItem.tsx | 46 ++- .../src/components/rhsChannelBoards.scss | 7 +- .../addContentMenuItem.test.tsx.snap | 32 +- .../blockIconSelector.test.tsx.snap | 24 +- .../__snapshots__/cardDialog.test.tsx.snap | 32 +- .../__snapshots__/contentBlock.test.tsx.snap | 32 +- .../__snapshots__/viewMenu.test.tsx.snap | 128 ++++-- .../cardActionsMenu.test.tsx.snap | 80 +++- .../cardDetailContentsMenu.test.tsx.snap | 120 ++++-- .../cardDetailProperties.test.tsx.snap | 120 ++++-- .../__snapshots__/comment.test.tsx.snap | 32 +- .../__snapshots__/gallery.test.tsx.snap | 48 ++- .../__snapshots__/galleryCard.test.tsx.snap | 160 ++++++-- .../globalHeaderSettingsMenu.test.tsx.snap | 248 ++++++++--- .../__snapshots__/kanbanCard.test.tsx.snap | 96 +++-- .../kanbanColumnHeader.test.tsx.snap | 24 +- .../kanbanHiddenColumnItem.test.tsx.snap | 32 +- .../teamPermissionsRow.test.tsx.snap | 104 +++-- .../sidebarSettingsMenu.test.tsx.snap | 304 ++++++++++---- .../tableHeaderMenu.test.tsx.snap | 104 +++-- .../emptyCardButton.test.tsx.snap | 40 +- .../filterComponent.test.tsx.snap | 184 ++++++--- .../__snapshots__/filterEntry.test.tsx.snap | 384 +++++++++++++----- .../__snapshots__/filterValue.test.tsx.snap | 24 +- .../__snapshots__/newCardButton.test.tsx.snap | 72 +++- .../newCardButtonTemplateItem.test.tsx.snap | 168 ++++++-- .../viewHeaderActionsMenu.test.tsx.snap | 72 +++- .../viewHeaderGroupByMenu.test.tsx.snap | 216 +++++++--- .../viewHeaderPropertiesMenu.test.tsx.snap | 16 +- .../viewHeaderSortMenu.test.tsx.snap | 64 ++- webapp/src/styles/_typography.scss | 5 + .../__snapshots__/propertyMenu.test.tsx.snap | 136 +++++-- webapp/src/widgets/menu/menu.scss | 21 + webapp/src/widgets/menu/textOption.tsx | 16 +- webapp/src/widgets/menuWrapper.scss | 7 +- 38 files changed, 2440 insertions(+), 801 deletions(-) diff --git a/mattermost-plugin/webapp/src/components/__snapshots__/rhsChannelBoardItem.test.tsx.snap b/mattermost-plugin/webapp/src/components/__snapshots__/rhsChannelBoardItem.test.tsx.snap index 9e6b24490..6171f4f2c 100644 --- a/mattermost-plugin/webapp/src/components/__snapshots__/rhsChannelBoardItem.test.tsx.snap +++ b/mattermost-plugin/webapp/src/components/__snapshots__/rhsChannelBoardItem.test.tsx.snap @@ -69,7 +69,7 @@ exports[`components/rhsChannelBoardItem render board with menu open 1`] = ` /> +
{ it('render board', async () => { + const board = createBoard() const state = { teams: { current: { @@ -22,8 +23,12 @@ describe('components/rhsChannelBoardItem', () => { display_name: 'Team name', }, }, + boards: { + myBoardMemberships: { + [board.id]: {userId: 'user_id_1', schemeAdmin: true}, + }, + } } - const board = createBoard() board.updateAt = 1657311058157 board.title = 'Test board' @@ -37,6 +42,7 @@ describe('components/rhsChannelBoardItem', () => { }) it('render board with menu open', async () => { + const board = createBoard() const state = { teams: { current: { @@ -45,8 +51,12 @@ describe('components/rhsChannelBoardItem', () => { display_name: 'Team name', }, }, + boards: { + myBoardMemberships: { + [board.id]: {userId: 'user_id_1', schemeAdmin: true}, + }, + } } - const board = createBoard() board.updateAt = 1657311058157 board.title = 'Test board' diff --git a/mattermost-plugin/webapp/src/components/rhsChannelBoardItem.tsx b/mattermost-plugin/webapp/src/components/rhsChannelBoardItem.tsx index 542fe3a38..679f68e53 100644 --- a/mattermost-plugin/webapp/src/components/rhsChannelBoardItem.tsx +++ b/mattermost-plugin/webapp/src/components/rhsChannelBoardItem.tsx @@ -15,7 +15,10 @@ import Menu from '../../../../webapp/src/widgets/menu' import MenuWrapper from '../../../../webapp/src/widgets/menuWrapper' import {SuiteWindow} from '../../../../webapp/src/types/index' +import {Permission} from '../../../../webapp/src/constants' + import './rhsChannelBoardItem.scss' +import BoardPermissionGate from '../../../../webapp/src/components/permissions/boardPermissionGate' const windowAny = (window as SuiteWindow) @@ -55,18 +58,41 @@ const RHSChannelBoardItem = (props: Props) => { }/> - } - onClick={() => { - onUnlinkBoard(board) - }} - /> + + } + onClick={() => { + onUnlinkBoard(board) + }} + /> + + + } + onClick={() => { + onUnlinkBoard(board) + }} + subText={intl.formatMessage({id: 'rhs-board-non-admin-msg', defaultMessage:'You are not an admin of the board'})} + /> +
diff --git a/mattermost-plugin/webapp/src/components/rhsChannelBoards.scss b/mattermost-plugin/webapp/src/components/rhsChannelBoards.scss index c5fc1a014..856a2bc81 100644 --- a/mattermost-plugin/webapp/src/components/rhsChannelBoards.scss +++ b/mattermost-plugin/webapp/src/components/rhsChannelBoards.scss @@ -1,9 +1,9 @@ .RHSChannelBoards { - padding: 20px; + padding: 16px 24px; height: 100%; display: flex; flex-direction: column; - gap: 10px; + gap: 16px; &.empty { display: flex; @@ -45,7 +45,8 @@ overflow-y: auto; display: flex; flex-direction: column; - gap: 10px; + gap: 16px; + min-height: 100%; } .Button { diff --git a/webapp/src/components/__snapshots__/addContentMenuItem.test.tsx.snap b/webapp/src/components/__snapshots__/addContentMenuItem.test.tsx.snap index 8f010d0e3..4f243aa13 100644 --- a/webapp/src/components/__snapshots__/addContentMenuItem.test.tsx.snap +++ b/webapp/src/components/__snapshots__/addContentMenuItem.test.tsx.snap @@ -21,9 +21,13 @@ exports[`components/addContentMenuItem return a checkbox menu item 1`] = `
.menu-content { + display: block; + } + > .menu-name { overflow: hidden; text-overflow: ellipsis; @@ -98,6 +114,11 @@ text-align: left; } + > .menu-subtext { + font-size: 10px; + text-align: left; + } + > .SubmenuTriangleIcon { fill: rgba(var(--center-channel-color-rgb), 0.7); } diff --git a/webapp/src/widgets/menu/textOption.tsx b/webapp/src/widgets/menu/textOption.tsx index f0e5d2b43..93dd31c04 100644 --- a/webapp/src/widgets/menu/textOption.tsx +++ b/webapp/src/widgets/menu/textOption.tsx @@ -9,14 +9,23 @@ type TextOptionProps = MenuOptionProps & { icon?: React.ReactNode, rightIcon?: React.ReactNode, className?: string + subText?: string + disabled?: boolean } function TextOption(props:TextOptionProps): JSX.Element { - const {name, icon, rightIcon, check} = props + const {name, icon, rightIcon, check, subText, disabled} = props let className = 'MenuOption TextOption menu-option' if (props.className) { className += ' ' + props.className } + if (subText) { + className += ' menu-option--with-subtext' + } + if (disabled) { + className += ' menu-option--disabled' + } + return (
{icon ??
}
-
{name}
+
+
{name}
+ {subText &&
{subText}
} +
{rightIcon ??
}
) diff --git a/webapp/src/widgets/menuWrapper.scss b/webapp/src/widgets/menuWrapper.scss index 39324b4a5..410e44b5e 100644 --- a/webapp/src/widgets/menuWrapper.scss +++ b/webapp/src/widgets/menuWrapper.scss @@ -1,12 +1,13 @@ .MenuWrapper { position: relative; - + cursor: default; + &.disabled { cursor: default; } - - *:first-child { + + *:first-child { /* stylelint-disable property-no-vendor-prefix*/ -webkit-user-select: text; /* Chrome all / Safari all */ -moz-user-select: text; /* Firefox all */ From 0403c22f3cbe944a9733cffebef5919a91c63b55 Mon Sep 17 00:00:00 2001 From: Scott Bishel Date: Tue, 23 Aug 2022 08:48:41 -0600 Subject: [PATCH 3/6] Post message channel change (#3717) * initial implementation * checking changes * use boards bot * update mocks * linter fixes * linter fixes * clean up * revert manifest change * another lint fix * use common error Co-authored-by: Mattermod --- .../server/boards/notifications.go | 18 ++----- server/app/boards.go | 39 +++++++++++++++ server/model/services_api.go | 13 +++++ .../mattermostauthlayer.go | 49 ++++++++++++------- server/services/store/mockstore/mockstore.go | 14 ++++++ .../services/store/sqlstore/public_methods.go | 5 ++ server/services/store/sqlstore/user.go | 4 ++ server/services/store/store.go | 1 + 8 files changed, 109 insertions(+), 34 deletions(-) diff --git a/mattermost-plugin/server/boards/notifications.go b/mattermost-plugin/server/boards/notifications.go index 62ab31735..a5de62a53 100644 --- a/mattermost-plugin/server/boards/notifications.go +++ b/mattermost-plugin/server/boards/notifications.go @@ -12,17 +12,9 @@ import ( "github.com/mattermost/focalboard/server/services/permissions" "github.com/mattermost/focalboard/server/services/store" - mm_model "github.com/mattermost/mattermost-server/v6/model" - "github.com/mattermost/mattermost-server/v6/shared/mlog" ) -const ( - botUsername = "boards" - botDisplayname = "Boards" - botDescription = "Created by Boards plugin." -) - type notifyBackendParams struct { cfg *config.Configuration servicesAPI model.ServicesAPI @@ -71,15 +63,11 @@ func createSubscriptionsNotifyBackend(params notifyBackendParams) (*notifysubscr } func createDelivery(servicesAPI model.ServicesAPI, serverRoot string) (*plugindelivery.PluginDelivery, error) { - bot := &mm_model.Bot{ - Username: botUsername, - DisplayName: botDisplayname, - Description: botDescription, - OwnerId: model.SystemUserID, - } + bot := model.FocalboardBot + botID, err := servicesAPI.EnsureBot(bot) if err != nil { - return nil, fmt.Errorf("failed to ensure %s bot: %w", botDisplayname, err) + return nil, fmt.Errorf("failed to ensure %s bot: %w", bot.DisplayName, err) } return plugindelivery.New(botID, serverRoot, servicesAPI), nil diff --git a/server/app/boards.go b/server/app/boards.go index 785039b16..0a1ca9dd4 100644 --- a/server/app/boards.go +++ b/server/app/boards.go @@ -20,6 +20,9 @@ var ( ErrInsufficientLicense = errors.New("appropriate license required") ) +const linkBoardMessage = "@%s linked Board [%s](%s) with this channel" +const unlinkBoardMessage = "@%s unlinked Board [%s](%s) with this channel" + func (a *App) GetBoard(boardID string) (*model.Board, error) { board, err := a.store.GetBoard(boardID) if model.IsErrNotFound(err) { @@ -302,18 +305,54 @@ func (a *App) CreateBoard(board *model.Board, userID string, addMember bool) (*m func (a *App) PatchBoard(patch *model.BoardPatch, boardID, userID string) (*model.Board, error) { var oldMembers []*model.BoardMember + var oldChannelID string if patch.ChannelID != nil && *patch.ChannelID == "" { var err error oldMembers, err = a.GetMembersForBoard(boardID) if err != nil { a.logger.Error("Unable to get the board members", mlog.Err(err)) } + board, err := a.store.GetBoard(boardID) + if model.IsErrNotFound(err) { + return nil, model.NewErrNotFound(boardID) + } + if err != nil { + return nil, err + } + oldChannelID = board.ChannelID } updatedBoard, err := a.store.PatchBoard(boardID, patch, userID) if err != nil { return nil, err } + if patch.ChannelID != nil { + var username string + user, err := a.store.GetUserByID(userID) + if err != nil { + a.logger.Error("Unable to get the board updater", mlog.Err(err)) + username = "unknown" + } else { + username = user.Username + } + + boardLink := utils.MakeBoardLink(a.config.ServerRoot, updatedBoard.TeamID, updatedBoard.ID) + if *patch.ChannelID != "" { + // TODO: this needs translated when available on the server + message := fmt.Sprintf(linkBoardMessage, username, updatedBoard.Title, boardLink) + err := a.store.PostMessage(message, "", *patch.ChannelID) + if err != nil { + a.logger.Error("Unable to post the link message to channel", mlog.Err(err)) + } + } else if *patch.ChannelID == "" { + message := fmt.Sprintf(unlinkBoardMessage, username, updatedBoard.Title, boardLink) + err := a.store.PostMessage(message, "", oldChannelID) + if err != nil { + a.logger.Error("Unable to post the link message to channel", mlog.Err(err)) + } + } + } + a.blockChangeNotifier.Enqueue(func() error { a.wsAdapter.BroadcastBoardChange(updatedBoard.TeamID, updatedBoard) if patch.ChannelID != nil && *patch.ChannelID != "" { diff --git a/server/model/services_api.go b/server/model/services_api.go index dcade8757..6bdd737e1 100644 --- a/server/model/services_api.go +++ b/server/model/services_api.go @@ -14,6 +14,19 @@ import ( "github.com/mattermost/mattermost-server/v6/shared/mlog" ) +const ( + botUsername = "boards" + botDisplayname = "Boards" + botDescription = "Created by Boards plugin." +) + +var FocalboardBot = &mm_model.Bot{ + Username: botUsername, + DisplayName: botDisplayname, + Description: botDescription, + OwnerId: SystemUserID, +} + type ServicesAPI interface { // Channels service GetDirectChannel(userID1, userID2 string) (*mm_model.Channel, error) diff --git a/server/services/store/mattermostauthlayer/mattermostauthlayer.go b/server/services/store/mattermostauthlayer/mattermostauthlayer.go index 52ad35152..7ad23c332 100644 --- a/server/services/store/mattermostauthlayer/mattermostauthlayer.go +++ b/server/services/store/mattermostauthlayer/mattermostauthlayer.go @@ -18,10 +18,7 @@ import ( "github.com/mattermost/mattermost-server/v6/shared/mlog" ) -var systemsBot = &mmModel.Bot{ - Username: mmModel.BotSystemBotUsername, - DisplayName: "System", -} +var boardsBotID string // servicesAPI is the interface required my the MattermostAuthLayer to interact with // the mattermost-server. You can use plugin-api or product-api adapter implementations. @@ -852,18 +849,18 @@ func (s *MattermostAuthLayer) GetChannel(teamID, channelID string) (*mmModel.Cha return channel, nil } -func (s *MattermostAuthLayer) getSystemBotID() (string, error) { - botID, err := s.servicesAPI.EnsureBot(systemsBot) - if err != nil { - s.logger.Error("failed to ensure system bot", mlog.String("username", systemsBot.Username), mlog.Err(err)) +func (s *MattermostAuthLayer) getBoardsBotID() (string, error) { + if boardsBotID == "" { + var err error + boardsBotID, err = s.servicesAPI.EnsureBot(model.FocalboardBot) + s.logger.Error("failed to ensure boards bot", mlog.Err(err)) return "", err } - - return botID, nil + return boardsBotID, nil } func (s *MattermostAuthLayer) SendMessage(message, postType string, receipts []string) error { - botID, err := s.getSystemBotID() + botID, err := s.getBoardsBotID() if err != nil { return err } @@ -880,14 +877,7 @@ func (s *MattermostAuthLayer) SendMessage(message, postType string, receipts []s continue } - post := &mmModel.Post{ - Message: message, - UserId: botID, - ChannelId: channel.Id, - Type: postType, - } - - if _, err := s.servicesAPI.CreatePost(post); err != nil { + if err := s.PostMessage(message, postType, channel.Id); err != nil { s.logger.Error( "failed to send message to receipt from SendMessage", mlog.String("receipt", receipt), @@ -896,7 +886,28 @@ func (s *MattermostAuthLayer) SendMessage(message, postType string, receipts []s continue } } + return nil +} +func (s *MattermostAuthLayer) PostMessage(message, postType, channelID string) error { + botID, err := s.getBoardsBotID() + if err != nil { + return err + } + + post := &mmModel.Post{ + Message: message, + UserId: botID, + ChannelId: channelID, + Type: postType, + } + + if _, err := s.servicesAPI.CreatePost(post); err != nil { + s.logger.Error( + "failed to send message to receipt from PostMessage", + mlog.Err(err), + ) + } return nil } diff --git a/server/services/store/mockstore/mockstore.go b/server/services/store/mockstore/mockstore.go index c56fa17ba..2b76efa72 100644 --- a/server/services/store/mockstore/mockstore.go +++ b/server/services/store/mockstore/mockstore.go @@ -1251,6 +1251,20 @@ func (mr *MockStoreMockRecorder) PatchUserProps(arg0, arg1 interface{}) *gomock. return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "PatchUserProps", reflect.TypeOf((*MockStore)(nil).PatchUserProps), arg0, arg1) } +// PostMessage mocks base method. +func (m *MockStore) PostMessage(arg0, arg1, arg2 string) error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "PostMessage", arg0, arg1, arg2) + ret0, _ := ret[0].(error) + return ret0 +} + +// PostMessage indicates an expected call of PostMessage. +func (mr *MockStoreMockRecorder) PostMessage(arg0, arg1, arg2 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "PostMessage", reflect.TypeOf((*MockStore)(nil).PostMessage), arg0, arg1, arg2) +} + // RefreshSession mocks base method. func (m *MockStore) RefreshSession(arg0 *model.Session) error { m.ctrl.T.Helper() diff --git a/server/services/store/sqlstore/public_methods.go b/server/services/store/sqlstore/public_methods.go index bd4435990..2c54070bc 100644 --- a/server/services/store/sqlstore/public_methods.go +++ b/server/services/store/sqlstore/public_methods.go @@ -707,6 +707,11 @@ func (s *SQLStore) PatchUserProps(userID string, patch model.UserPropPatch) erro } +func (s *SQLStore) PostMessage(message string, postType string, channelID string) error { + return s.postMessage(s.db, message, postType, channelID) + +} + func (s *SQLStore) RefreshSession(session *model.Session) error { return s.refreshSession(s.db, session) diff --git a/server/services/store/sqlstore/user.go b/server/services/store/sqlstore/user.go index 3fcf9b6cb..85d57516b 100644 --- a/server/services/store/sqlstore/user.go +++ b/server/services/store/sqlstore/user.go @@ -279,6 +279,10 @@ func (s *SQLStore) sendMessage(db sq.BaseRunner, message, postType string, recei return errUnsupportedOperation } +func (s *SQLStore) postMessage(db sq.BaseRunner, message, postType string, channel string) error { + return errUnsupportedOperation +} + func (s *SQLStore) getUserTimezone(_ sq.BaseRunner, _ string) (string, error) { return "", errUnsupportedOperation } diff --git a/server/services/store/store.go b/server/services/store/store.go index 87c3daa9f..637713657 100644 --- a/server/services/store/store.go +++ b/server/services/store/store.go @@ -154,6 +154,7 @@ type Store interface { GetCloudLimits() (*mmModel.ProductLimits, error) SearchUserChannels(teamID, userID, query string) ([]*mmModel.Channel, error) GetChannel(teamID, channelID string) (*mmModel.Channel, error) + PostMessage(message, postType, channelID string) error SendMessage(message, postType string, receipts []string) error // Insights From bfde68c53208597146ee8704bd8f4d142a06614c Mon Sep 17 00:00:00 2001 From: Rajat Dabade Date: Tue, 23 Aug 2022 20:20:18 +0530 Subject: [PATCH 4/6] Fixed the new button error in kanban (#3696) * Fixed the new button error in kanban * Test cases fixed * Updated test * Update tests * Code update for cypress test * Update acrding review changes * Updated dependency of the useEffect * Code update * Changed the condition in useEffect * Revert "Update acrding review changes" This reverts commit 207f95fcda220cafc2d33fa069478c366559404e. * Revert Updated dependency of the useEffect * Replaced currentView by activeView --- .../boardTemplateSelectorPreview.test.tsx | 26 ++++++++++++------- webapp/src/components/kanban/kanban.test.tsx | 7 +++-- webapp/src/components/kanban/kanban.tsx | 19 ++++++++++---- 3 files changed, 36 insertions(+), 16 deletions(-) diff --git a/webapp/src/components/boardTemplateSelector/boardTemplateSelectorPreview.test.tsx b/webapp/src/components/boardTemplateSelector/boardTemplateSelectorPreview.test.tsx index 19ef32497..f18c2d087 100644 --- a/webapp/src/components/boardTemplateSelector/boardTemplateSelectorPreview.test.tsx +++ b/webapp/src/components/boardTemplateSelector/boardTemplateSelectorPreview.test.tsx @@ -9,6 +9,8 @@ import {Provider as ReduxProvider} from 'react-redux' import {IPropertyTemplate} from '../../blocks/board' import {mockDOM, mockStateStore, wrapDNDIntl} from '../../testUtils' +import {TestBlockFactory} from '../../test/testBlockFactory' + import BoardTemplateSelectorPreview from './boardTemplateSelectorPreview' jest.mock('react-router-dom', () => { @@ -94,14 +96,15 @@ describe('components/boardTemplateSelector/boardTemplateSelectorPreview', () => beforeAll(mockDOM) beforeEach(() => { jest.clearAllMocks() - const board = { - id: '2', - title: boardTitle, - teamId: 'team-id', - icon: '🚴🏻‍♂️', - cardProperties: [groupProperty], - dateDisplayPropertyId: 'id-6', - } + + const board = TestBlockFactory.createBoard() + board.id = '2' + board.title = boardTitle + board.teamId = 'team-id' + board.icon = '🚴🏻‍♂️' + board.cardProperties = [groupProperty] + const activeView = TestBlockFactory.createBoardView(board) + activeView.fields.defaultTemplateId = 'defaultTemplateId' const state = { searchText: {value: ''}, @@ -120,7 +123,12 @@ describe('components/boardTemplateSelector/boardTemplateSelectorPreview', () => }, current: 'card_id_1', }, - views: {views: []}, + views: { + views: { + boardView: activeView + }, + current: 'boardView' + }, contents: {contents: []}, comments: {comments: []}, teams: { diff --git a/webapp/src/components/kanban/kanban.test.tsx b/webapp/src/components/kanban/kanban.test.tsx index 98fc06956..7566fd942 100644 --- a/webapp/src/components/kanban/kanban.test.tsx +++ b/webapp/src/components/kanban/kanban.test.tsx @@ -75,6 +75,7 @@ describe('src/component/kanban/kanban', () => { }, cards: { cards: [card1, card2, card3], + templates: [], }, teams: { current: {id: 'team-id'}, @@ -542,6 +543,7 @@ describe('src/component/kanban/kanban', () => { card2.fields.properties = {id: 'property_value_id_1'} const card3 = TestBlockFactory.createCard(board) card3.id = 'id3' + card3.boardId = 'board_id_1' card3.fields.properties = {id: 'property_value_id_2'} activeView.fields.kanbanCalculations = { id1: { @@ -550,7 +552,7 @@ describe('src/component/kanban/kanban', () => { }, } - activeView.fields.defaultTemplateId = "defaultTemplateId" + activeView.fields.defaultTemplateId = card3.id const optionQ1:IPropertyOption = { color: 'propColorOrange', id: 'property_value_id_1', @@ -582,7 +584,8 @@ describe('src/component/kanban/kanban', () => { }, }, cards: { - cards: [card1, card2, card3], + cards: [card1, card2], + templates: [card3], }, teams: { current: {id: 'team-id'}, diff --git a/webapp/src/components/kanban/kanban.tsx b/webapp/src/components/kanban/kanban.tsx index 81bde2d15..ff1d8d742 100644 --- a/webapp/src/components/kanban/kanban.tsx +++ b/webapp/src/components/kanban/kanban.tsx @@ -1,13 +1,12 @@ // Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. // See LICENSE.txt for license information. /* eslint-disable max-lines */ -import React, {useCallback, useState, useMemo} from 'react' +import React, { useCallback, useState, useMemo, useEffect } from 'react' import {FormattedMessage, injectIntl, IntlShape} from 'react-intl' import withScrolling, {createHorizontalStrength, createVerticalStrength} from 'react-dnd-scrolling' import {useAppSelector} from '../../store/hooks' -import {getCurrentView} from '../../store/views' import {Position} from '../cardDetail/cardDetailContents' @@ -21,6 +20,7 @@ import {Constants, Permission} from '../../constants' import {dragAndDropRearrange} from '../cardDetail/cardDetailContentsUtility' +import {getCurrentBoardTemplates} from '../../store/cards' import BoardPermissionGate from '../permissions/boardPermissionGate' import HiddenCardCount from '../../components/hiddenCardCount/hiddenCardCount' @@ -54,8 +54,17 @@ const hStrength = createHorizontalStrength(Utils.isMobile() ? 60 : 250) const vStrength = createVerticalStrength(Utils.isMobile() ? 60 : 250) const Kanban = (props: Props) => { - const currentView = useAppSelector(getCurrentView) + const cardTemplates: Card[] = useAppSelector(getCurrentBoardTemplates) const {board, activeView, cards, groupByProperty, visibleGroups, hiddenGroups, hiddenCardsCount} = props + const [defaultTemplateID, setDefaultTemplateID] = useState() + + useEffect(() => { + if(activeView.fields.defaultTemplateId) { + if(cardTemplates.find(ct => ct.id === activeView.fields.defaultTemplateId)) { + setDefaultTemplateID(activeView.fields.defaultTemplateId) + } + } + }, [activeView.fields.defaultTemplateId]) if (!groupByProperty) { Utils.assertFailure('Board views must have groupByProperty set') @@ -297,8 +306,8 @@ const Kanban = (props: Props) => {