From 50930a1cd3f0abadd370cb97b5d8b8def1c7c3ce Mon Sep 17 00:00:00 2001 From: Harshil Sharma <18575143+harshilsharma63@users.noreply.github.com> Date: Tue, 19 Apr 2022 10:27:14 +0530 Subject: [PATCH] Cross team search (#2829) * WIP * Added test * Added snapshot * Updated server tests * Fixed server lint * Fixed webapp lint * Updated server tests --- server/api/api.go | 2 +- server/app/boards.go | 4 +- server/integrationtests/board_test.go | 8 +- server/services/store/mockstore/mockstore.go | 12 +-- server/services/store/sqlstore/board.go | 7 +- .../services/store/sqlstore/public_methods.go | 4 +- server/services/store/store.go | 2 +- server/services/store/storetests/boards.go | 23 ++--- .../boardSwitcherDialog.test.tsx.snap | 95 +++++++++++++++++++ .../boardSwitcherDialog.scss | 11 +++ .../boardSwitcherDialog.test.tsx | 76 +++++++++++++++ .../boardSwitcherDialog.tsx | 36 ++++--- webapp/src/store/teams.ts | 1 + webapp/src/test/testBlockFactory.ts | 13 +++ 14 files changed, 245 insertions(+), 49 deletions(-) create mode 100644 webapp/src/components/boardsSwitcherDialog/__snapshots__/boardSwitcherDialog.test.tsx.snap create mode 100644 webapp/src/components/boardsSwitcherDialog/boardSwitcherDialog.test.tsx diff --git a/server/api/api.go b/server/api/api.go index 38a304184..f2bed6dd6 100644 --- a/server/api/api.go +++ b/server/api/api.go @@ -3166,7 +3166,7 @@ func (a *API) handleSearchBoards(w http.ResponseWriter, r *http.Request) { auditRec.AddMeta("teamID", teamID) // retrieve boards list - boards, err := a.app.SearchBoardsForUserAndTeam(term, userID, teamID) + boards, err := a.app.SearchBoardsForUser(term, userID, teamID) if err != nil { a.errorResponse(w, r.URL.Path, http.StatusInternalServerError, "", err) return diff --git a/server/app/boards.go b/server/app/boards.go index 70547b78a..21fdf8db3 100644 --- a/server/app/boards.go +++ b/server/app/boards.go @@ -369,8 +369,8 @@ func (a *App) DeleteBoardMember(boardID, userID string) error { return nil } -func (a *App) SearchBoardsForUserAndTeam(term, userID, teamID string) ([]*model.Board, error) { - return a.store.SearchBoardsForUserAndTeam(term, userID, teamID) +func (a *App) SearchBoardsForUser(term, userID, teamID string) ([]*model.Board, error) { + return a.store.SearchBoardsForUser(term, userID, teamID) } func (a *App) UndeleteBoard(boardID string, modifiedBy string) error { diff --git a/server/integrationtests/board_test.go b/server/integrationtests/board_test.go index c9a5405ec..3dc5fb78a 100644 --- a/server/integrationtests/board_test.go +++ b/server/integrationtests/board_test.go @@ -543,25 +543,25 @@ func TestSearchBoards(t *testing.T) { Name: "should return all boards where user1 is member or that are public", Client: th.Client, Term: "board", - ExpectedIDs: []string{rBoard1.ID, rBoard2.ID, rBoard3.ID}, + ExpectedIDs: []string{rBoard1.ID, rBoard2.ID, rBoard3.ID, board5.ID}, }, { Name: "matching a full word", Client: th.Client, Term: "admin", - ExpectedIDs: []string{rBoard1.ID, rBoard3.ID}, + ExpectedIDs: []string{rBoard1.ID, rBoard3.ID, board5.ID}, }, { Name: "matching part of the word", Client: th.Client, Term: "ubli", - ExpectedIDs: []string{rBoard1.ID, rBoard2.ID}, + ExpectedIDs: []string{rBoard1.ID, rBoard2.ID, board5.ID}, }, { Name: "case insensitive", Client: th.Client, Term: "UBLI", - ExpectedIDs: []string{rBoard1.ID, rBoard2.ID}, + ExpectedIDs: []string{rBoard1.ID, rBoard2.ID, board5.ID}, }, { Name: "user2 can only see the public boards, as he's not a member of any", diff --git a/server/services/store/mockstore/mockstore.go b/server/services/store/mockstore/mockstore.go index ce6928e3c..a9417810b 100644 --- a/server/services/store/mockstore/mockstore.go +++ b/server/services/store/mockstore/mockstore.go @@ -1158,19 +1158,19 @@ func (mr *MockStoreMockRecorder) SaveMember(arg0 interface{}) *gomock.Call { return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SaveMember", reflect.TypeOf((*MockStore)(nil).SaveMember), arg0) } -// SearchBoardsForUserAndTeam mocks base method. -func (m *MockStore) SearchBoardsForUserAndTeam(arg0, arg1, arg2 string) ([]*model.Board, error) { +// SearchBoardsForUser mocks base method. +func (m *MockStore) SearchBoardsForUser(arg0, arg1, arg2 string) ([]*model.Board, error) { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "SearchBoardsForUserAndTeam", arg0, arg1, arg2) + ret := m.ctrl.Call(m, "SearchBoardsForUser", arg0, arg1, arg2) ret0, _ := ret[0].([]*model.Board) ret1, _ := ret[1].(error) return ret0, ret1 } -// SearchBoardsForUserAndTeam indicates an expected call of SearchBoardsForUserAndTeam. -func (mr *MockStoreMockRecorder) SearchBoardsForUserAndTeam(arg0, arg1, arg2 interface{}) *gomock.Call { +// SearchBoardsForUser indicates an expected call of SearchBoardsForUser. +func (mr *MockStoreMockRecorder) SearchBoardsForUser(arg0, arg1, arg2 interface{}) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SearchBoardsForUserAndTeam", reflect.TypeOf((*MockStore)(nil).SearchBoardsForUserAndTeam), arg0, arg1, arg2) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SearchBoardsForUser", reflect.TypeOf((*MockStore)(nil).SearchBoardsForUser), arg0, arg1, arg2) } // SearchUsersByTeam mocks base method. diff --git a/server/services/store/sqlstore/board.go b/server/services/store/sqlstore/board.go index 9e1116bce..feae7d588 100644 --- a/server/services/store/sqlstore/board.go +++ b/server/services/store/sqlstore/board.go @@ -597,17 +597,16 @@ func (s *SQLStore) getMembersForBoard(db sq.BaseRunner, boardID string) ([]*mode return s.boardMembersFromRows(rows) } -// searchBoardsForUserAndTeam returns all boards that match with the +// searchBoardsForUser returns all boards that match with the // term that are either private and which the user is a member of, or // they're open, regardless of the user membership. // Search is case-insensitive. -func (s *SQLStore) searchBoardsForUserAndTeam(db sq.BaseRunner, term, userID, teamID string) ([]*model.Board, error) { +func (s *SQLStore) searchBoardsForUser(db sq.BaseRunner, term, userID string) ([]*model.Board, error) { query := s.getQueryBuilder(db). Select(boardFields("b.")...). Distinct(). From(s.tablePrefix + "boards as b"). LeftJoin(s.tablePrefix + "board_members as bm on b.id=bm.board_id"). - Where(sq.Eq{"b.team_id": teamID}). Where(sq.Eq{"b.is_template": false}). Where(sq.Or{ sq.Eq{"b.type": model.BoardTypeOpen}, @@ -635,7 +634,7 @@ func (s *SQLStore) searchBoardsForUserAndTeam(db sq.BaseRunner, term, userID, te rows, err := query.Query() if err != nil { - s.logger.Error(`searchBoardsForUserAndTeam ERROR`, mlog.Err(err)) + s.logger.Error(`searchBoardsForUser ERROR`, mlog.Err(err)) return nil, err } defer s.CloseRows(rows) diff --git a/server/services/store/sqlstore/public_methods.go b/server/services/store/sqlstore/public_methods.go index 29acd8ca9..d6e9373f9 100644 --- a/server/services/store/sqlstore/public_methods.go +++ b/server/services/store/sqlstore/public_methods.go @@ -677,8 +677,8 @@ func (s *SQLStore) SaveMember(bm *model.BoardMember) (*model.BoardMember, error) } -func (s *SQLStore) SearchBoardsForUserAndTeam(term string, userID string, teamID string) ([]*model.Board, error) { - return s.searchBoardsForUserAndTeam(s.db, term, userID, teamID) +func (s *SQLStore) SearchBoardsForUser(term string, userID string, teamID string) ([]*model.Board, error) { + return s.searchBoardsForUser(s.db, term, userID) } diff --git a/server/services/store/store.go b/server/services/store/store.go index e7f953541..8530f2bd7 100644 --- a/server/services/store/store.go +++ b/server/services/store/store.go @@ -99,7 +99,7 @@ type Store interface { GetBoardMemberHistory(boardID, userID string, limit uint64) ([]*model.BoardMemberHistoryEntry, error) GetMembersForBoard(boardID string) ([]*model.BoardMember, error) GetMembersForUser(userID string) ([]*model.BoardMember, error) - SearchBoardsForUserAndTeam(term, userID, teamID string) ([]*model.Board, error) + SearchBoardsForUser(term, userID, teamID string) ([]*model.Board, error) // @withTransaction CreateBoardsAndBlocksWithAdmin(bab *model.BoardsAndBlocks, userID string) (*model.BoardsAndBlocks, []*model.BoardMember, error) diff --git a/server/services/store/storetests/boards.go b/server/services/store/storetests/boards.go index e04f07510..0de69210a 100644 --- a/server/services/store/storetests/boards.go +++ b/server/services/store/storetests/boards.go @@ -68,10 +68,10 @@ func StoreTestBoardStore(t *testing.T, setup func(t *testing.T) (store.Store, fu defer tearDown() testDeleteMember(t, store) }) - t.Run("SearchBoardsForUserAndTeam", func(t *testing.T) { + t.Run("SearchBoardsForUser", func(t *testing.T) { store, tearDown := setup(t) defer tearDown() - testSearchBoardsForUserAndTeam(t, store) + testSearchBoardsForUser(t, store) }) t.Run("GetBoardHistory", func(t *testing.T) { store, tearDown := setup(t) @@ -683,13 +683,13 @@ func testDeleteMember(t *testing.T, store store.Store) { }) } -func testSearchBoardsForUserAndTeam(t *testing.T, store store.Store) { +func testSearchBoardsForUser(t *testing.T, store store.Store) { teamID1 := "team-id-1" teamID2 := "team-id-2" userID := "user-id-1" t.Run("should return empty if user is not a member of any board and there are no public boards on the team", func(t *testing.T) { - boards, err := store.SearchBoardsForUserAndTeam("", userID, teamID1) + boards, err := store.SearchBoardsForUser("", userID, teamID1) require.NoError(t, err) require.Empty(t, boards) }) @@ -751,21 +751,21 @@ func testSearchBoardsForUserAndTeam(t *testing.T, store store.Store) { TeamID: teamID1, UserID: userID, Term: "", - ExpectedBoardIDs: []string{board1.ID, board2.ID, board3.ID}, + ExpectedBoardIDs: []string{board1.ID, board2.ID, board3.ID, board5.ID}, }, { Name: "should find all with term board", TeamID: teamID1, UserID: userID, Term: "board", - ExpectedBoardIDs: []string{board1.ID, board2.ID, board3.ID}, + ExpectedBoardIDs: []string{board1.ID, board2.ID, board3.ID, board5.ID}, }, { Name: "should find only public as per the term, wether user is a member or not", TeamID: teamID1, UserID: userID, Term: "public", - ExpectedBoardIDs: []string{board1.ID, board2.ID}, + ExpectedBoardIDs: []string{board1.ID, board2.ID, board5.ID}, }, { Name: "should find only private as per the term, wether user is a member or not", @@ -774,13 +774,6 @@ func testSearchBoardsForUserAndTeam(t *testing.T, store store.Store) { Term: "priv", ExpectedBoardIDs: []string{board3.ID}, }, - { - Name: "should find the only board in team 2", - TeamID: teamID2, - UserID: userID, - Term: "", - ExpectedBoardIDs: []string{board5.ID}, - }, { Name: "should find no board in team 2 with a non matching term", TeamID: teamID2, @@ -792,7 +785,7 @@ func testSearchBoardsForUserAndTeam(t *testing.T, store store.Store) { for _, tc := range testCases { t.Run(tc.Name, func(t *testing.T) { - boards, err := store.SearchBoardsForUserAndTeam(tc.Term, tc.UserID, tc.TeamID) + boards, err := store.SearchBoardsForUser(tc.Term, tc.UserID, tc.TeamID) require.NoError(t, err) boardIDs := []string{} diff --git a/webapp/src/components/boardsSwitcherDialog/__snapshots__/boardSwitcherDialog.test.tsx.snap b/webapp/src/components/boardsSwitcherDialog/__snapshots__/boardSwitcherDialog.test.tsx.snap new file mode 100644 index 000000000..59b5ccc1f --- /dev/null +++ b/webapp/src/components/boardsSwitcherDialog/__snapshots__/boardSwitcherDialog.test.tsx.snap @@ -0,0 +1,95 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`component/BoardSwitcherDialog base case 1`] = ` +
+
+
+ +
+
+
+`; diff --git a/webapp/src/components/boardsSwitcherDialog/boardSwitcherDialog.scss b/webapp/src/components/boardsSwitcherDialog/boardSwitcherDialog.scss index e856e164a..c1cf290b7 100644 --- a/webapp/src/components/boardsSwitcherDialog/boardSwitcherDialog.scss +++ b/webapp/src/components/boardsSwitcherDialog/boardSwitcherDialog.scss @@ -3,6 +3,7 @@ gap: 12px; overflow: hidden; flex-direction: row; + width: 100%; .CompassIcon { font-size: 18px; @@ -20,4 +21,14 @@ text-overflow: ellipsis; white-space: nowrap; } + + .resultTitle { + max-width: 60%; + } + + .teamTitle { + right: auto; + margin-left: auto; + color: rgba(var(--center-channel-color-rgb), 0.56); + } } diff --git a/webapp/src/components/boardsSwitcherDialog/boardSwitcherDialog.test.tsx b/webapp/src/components/boardsSwitcherDialog/boardSwitcherDialog.test.tsx new file mode 100644 index 000000000..75e9ae4a5 --- /dev/null +++ b/webapp/src/components/boardsSwitcherDialog/boardSwitcherDialog.test.tsx @@ -0,0 +1,76 @@ +// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. +// See LICENSE.txt for license information. + +import React from 'react' + +import {MockStoreEnhanced} from "redux-mock-store" + +import {Provider as ReduxProvider} from 'react-redux' + +import {render} from "@testing-library/react" + +import {createMemoryHistory, History} from "history" + +import {Router} from "react-router-dom" + +import {Team} from "../../store/teams" +import {TestBlockFactory} from "../../test/testBlockFactory" + +import {mockStateStore, wrapDNDIntl} from "../../testUtils" + +import BoardSwitcherDialog from "./boardSwitcherDialog" + + +describe('component/BoardSwitcherDialog', () => { + const team1: Team = { + id: 'team-id-1', + title: 'Dunder Mifflin', + signupToken: '', + updateAt: 0, + modifiedBy: 'michael-scott', + } + + const team2: Team = { + id: 'team-id-2', + title: 'Michael Scott Paper Company', + signupToken: '', + updateAt: 0, + modifiedBy: 'michael-scott', + } + + const me = TestBlockFactory.createUser() + + const state = { + users: { + me: me, + }, + teams: { + allTeams: [team1, team2], + current: team1, + } + } + + let store:MockStoreEnhanced + let history: History + + + beforeEach(() => { + store = mockStateStore([], state) + history = createMemoryHistory() + }) + + + test('base case', () => { + const onCloseHandler = jest.fn() + const component = wrapDNDIntl( + + + + + + ) + + const {container} = render(component) + expect(container).toMatchSnapshot() + }) +}) diff --git a/webapp/src/components/boardsSwitcherDialog/boardSwitcherDialog.tsx b/webapp/src/components/boardsSwitcherDialog/boardSwitcherDialog.tsx index 51498cccc..419eeba17 100644 --- a/webapp/src/components/boardsSwitcherDialog/boardSwitcherDialog.tsx +++ b/webapp/src/components/boardsSwitcherDialog/boardSwitcherDialog.tsx @@ -12,7 +12,7 @@ import SearchDialog from '../searchDialog/searchDialog' import Globe from '../../widgets/icons/globe' import LockOutline from '../../widgets/icons/lockOutline' import {useAppSelector} from '../../store/hooks' -import {getCurrentTeam} from '../../store/teams' +import {getAllTeams, getCurrentTeam, Team} from '../../store/teams' import {getMe} from '../../store/users' import {BoardTypeOpen, BoardTypePrivate} from '../../blocks/board' @@ -38,15 +38,18 @@ const BoardSwitcherDialog = (props: Props): JSX.Element => { const match = useRouteMatch<{boardId: string, viewId: string, cardId?: string}>() const history = useHistory() - const selectBoard = async (boardId: string): Promise => { + const selectBoard = async (teamId: string, boardId: string): Promise => { if (!me) { return } - const newPath = generatePath(match.path, {...match.params, boardId, viewId: undefined}) + const newPath = generatePath(match.path, {...match.params, teamId, boardId, viewId: undefined}) history.push(newPath) props.onClose() } + const teamsById:Record = {} + useAppSelector(getAllTeams).forEach((t) => teamsById[t.id] = t) + const searchHandler = async (query: string): Promise> => { if (query.trim().length === 0 || !team) { return [] @@ -54,17 +57,22 @@ const BoardSwitcherDialog = (props: Props): JSX.Element => { const items = await octoClient.search(team.id, query) const untitledBoardTitle = intl.formatMessage({id: 'ViewTitle.untitled-board', defaultMessage: 'Untitled Board'}) - return items.map((item) => ( -
selectBoard(item.id)} - > - {item.type === BoardTypeOpen && } - {item.type === BoardTypePrivate && } - {item.title || untitledBoardTitle} -
- )) + return items.map((item) => { + const resultTitle = item.title || untitledBoardTitle + const teamTitle = teamsById[item.teamId].title + return ( +
selectBoard(item.teamId, item.id)} + > + {item.type === BoardTypeOpen && } + {item.type === BoardTypePrivate && } + {resultTitle} + {teamTitle} +
+ ) + }) } return ( diff --git a/webapp/src/store/teams.ts b/webapp/src/store/teams.ts index ece837f92..3d2d781c8 100644 --- a/webapp/src/store/teams.ts +++ b/webapp/src/store/teams.ts @@ -82,3 +82,4 @@ export const {reducer} = teamSlice export const getCurrentTeam = (state: RootState): Team|null => state.teams.current export const getFirstTeam = (state: RootState): Team|null => state.teams.allTeams[0] +export const getAllTeams = (state: RootState): Array => state.teams.allTeams diff --git a/webapp/src/test/testBlockFactory.ts b/webapp/src/test/testBlockFactory.ts index a52956fd4..68bf21d05 100644 --- a/webapp/src/test/testBlockFactory.ts +++ b/webapp/src/test/testBlockFactory.ts @@ -14,6 +14,7 @@ import {Category, CategoryBoards} from '../store/sidebar' import {Utils} from '../utils' import {CheckboxBlock, createCheckboxBlock} from '../blocks/checkboxBlock' import {Block} from '../blocks/block' +import {IUser} from "../user" class TestBlockFactory { static createBoard(): Board { @@ -181,6 +182,18 @@ class TestBlockFactory { boardIDs: [], } } + + static createUser(): IUser { + return { + id: 'user-id-1', + username: 'Dwight Schrute', + email: 'dwight.schrute@dundermifflin.com', + props: {}, + create_at: Date.now(), + update_at: Date.now(), + is_bot: false, + } + } } export {TestBlockFactory}