diff --git a/.changeset/thick-fans-leave.md b/.changeset/thick-fans-leave.md new file mode 100644 index 0000000000000..e53d5041886ff --- /dev/null +++ b/.changeset/thick-fans-leave.md @@ -0,0 +1,5 @@ +--- +'@rocket.chat/meteor': patch +--- + +Fixes an issue where the channel selection modal would not load when removing a member from a team. diff --git a/apps/meteor/client/components/ResultsLiveRegion.tsx b/apps/meteor/client/components/ResultsLiveRegion.tsx index 3df599c66a509..7fc92094fb503 100644 --- a/apps/meteor/client/components/ResultsLiveRegion.tsx +++ b/apps/meteor/client/components/ResultsLiveRegion.tsx @@ -1,14 +1,28 @@ import { VisuallyHidden } from '@react-aria/visually-hidden'; import { useTranslation } from 'react-i18next'; -const ResultsLiveRegion = ({ shouldAnnounce, itemCount }: { shouldAnnounce: boolean; itemCount: number }) => { +const ResultsLiveRegion = ({ + shouldAnnounce, + itemCount, + isLoading = false, +}: { + shouldAnnounce: boolean; + itemCount: number; + isLoading?: boolean; +}) => { const { t } = useTranslation(); - if (itemCount === 0) { - return {shouldAnnounce && t('No_results_found')}; - } + const message = (() => { + if (isLoading) { + return t('Loading'); + } + if (!shouldAnnounce) { + return ''; + } + return itemCount === 0 ? t('No_results_found') : t('__count__result_found', { count: itemCount }); + })(); - return {shouldAnnounce && t('__count__result_found', { count: itemCount })}; + return {message}; }; export default ResultsLiveRegion; diff --git a/apps/meteor/client/navbar/NavBarSearch/NavBarSearchItemSkeleton.tsx b/apps/meteor/client/navbar/NavBarSearch/NavBarSearchItemSkeleton.tsx new file mode 100644 index 0000000000000..a99a3c9dcdb62 --- /dev/null +++ b/apps/meteor/client/navbar/NavBarSearch/NavBarSearchItemSkeleton.tsx @@ -0,0 +1,20 @@ +import { SidebarV2Item, SidebarV2ItemAvatarWrapper, SidebarV2ItemTitle, Skeleton } from '@rocket.chat/fuselage'; + +// Placeholder row shown while the server spotlight results are still loading. +// Decorative only: it has no `role='option'` so keyboard navigation skips it +// (see useSearchNavigation), and `tabIndex={-1}` + `pointerEvents='none'` keep +// it out of reach of focus and mouse clicks. Loading is conveyed via aria-busy. +const NavBarSearchItemSkeleton = () => { + return ( + + + + + + + + + ); +}; + +export default NavBarSearchItemSkeleton; diff --git a/apps/meteor/client/navbar/NavBarSearch/NavBarSearchListbox.tsx b/apps/meteor/client/navbar/NavBarSearch/NavBarSearchListbox.tsx index 1b3ef415fa414..5e02cd8412d86 100644 --- a/apps/meteor/client/navbar/NavBarSearch/NavBarSearchListbox.tsx +++ b/apps/meteor/client/navbar/NavBarSearch/NavBarSearchListbox.tsx @@ -1,12 +1,13 @@ import type { OverlayTriggerAria } from '@react-aria/overlays'; import type { OverlayTriggerState } from '@react-stately/overlays'; import { Box, Tile } from '@rocket.chat/fuselage'; -import { useDebouncedValue, useStableCallback, useOutsideClick } from '@rocket.chat/fuselage-hooks'; +import { useStableCallback, useOutsideClick } from '@rocket.chat/fuselage-hooks'; import { CustomScrollbars } from '@rocket.chat/ui-client'; import { useRef } from 'react'; import { useFormContext } from 'react-hook-form'; import { useTranslation } from 'react-i18next'; +import NavBarSearchItemSkeleton from './NavBarSearchItemSkeleton'; import NavBarSearchNoResults from './NavBarSearchNoResults'; import NavBarSearchRow from './NavBarSearchRow'; import { useSearchItems } from './hooks/useSearchItems'; @@ -28,14 +29,12 @@ const NavBarSearchListBox = ({ state, overlayProps }: NavBarSearchListBoxProps) const { resetField, watch } = useFormContext(); const { filterText } = watch(); - const debouncedFilter = useDebouncedValue(filterText, 500); - const handleSelect = useStableCallback(() => { state.close(); resetField('filterText'); }); - const { data: items = [], isLoading } = useSearchItems(debouncedFilter); + const { items, isLoading } = useSearchItems(filterText); return ( - +
{items.length === 0 && !isLoading && } @@ -63,6 +62,7 @@ const NavBarSearchListBox = ({ state, overlayProps }: NavBarSearchListBoxProps) {items.map((item) => ( ))} + {isLoading && Array.from({ length: 4 }, (_, index) => )}
diff --git a/apps/meteor/client/navbar/NavBarSearch/hooks/useSearchItems.ts b/apps/meteor/client/navbar/NavBarSearch/hooks/useSearchItems.ts index 97b6b077d5b52..1804e054a95ff 100644 --- a/apps/meteor/client/navbar/NavBarSearch/hooks/useSearchItems.ts +++ b/apps/meteor/client/navbar/NavBarSearch/hooks/useSearchItems.ts @@ -1,7 +1,8 @@ +import { useDebouncedValue } from '@rocket.chat/fuselage-hooks'; import { escapeRegExp } from '@rocket.chat/string-helpers'; import type { SubscriptionWithRoom } from '@rocket.chat/ui-contexts'; import { useEndpoint, useUserSubscriptions } from '@rocket.chat/ui-contexts'; -import { useQuery, type UseQueryResult } from '@tanstack/react-query'; +import { useQuery } from '@tanstack/react-query'; import { useMemo } from 'react'; import { getConfig } from '../../../lib/utils/getConfig'; @@ -16,9 +17,9 @@ const options = { limit: LIMIT, } as const; -// FIXME: the return type is UTTERLY wrong, but I'm not sure what it should be -export const useSearchItems = (filterText: string): UseQueryResult => { +export const useSearchItems = (filterText: string): { items: SubscriptionWithRoom[]; isLoading: boolean } => { const [, mention, name] = useMemo(() => filterText.match(/(@|#)?(.*)/i) || [], [filterText]); + const query = useMemo(() => { const filterRegex = new RegExp(escapeRegExp(name), 'i'); @@ -30,8 +31,13 @@ export const useSearchItems = (filterText: string): UseQueryResult (t === 'd' ? name : null))].filter(Boolean) as string[]; const searchForChannels = mention === '#'; @@ -49,16 +55,20 @@ export const useSearchItems = (filterText: string): UseQueryResult _id + name)], + const { + data: serverResults, + isFetching, + isPlaceholderData, + } = useQuery({ + // Keyed on the debounced term only, so typing doesn't refetch on every keystroke. + queryKey: ['sidebar/search/spotlight', debouncedName, mention, type], - queryFn: async () => { - if (localRooms.length === LIMIT) { - return localRooms; - } + // When local subscriptions already fill the limit there's nothing more to fetch. + enabled: localRooms.length < LIMIT, + queryFn: async () => { const spotlight = await getSpotlight({ - query: name, + query: debouncedName, usernames: usernamesFromClient.join(','), type: JSON.stringify(type), }); @@ -66,15 +76,6 @@ export const useSearchItems = (filterText: string): UseQueryResult index === arr.findIndex((user) => _id === user._id); - const roomFilter = (room: { t: string; uids?: string[]; _id: string; name?: string }): boolean => - !localRooms.find( - (item) => - (room.t === 'd' && room.uids && room.uids.length > 1 && room.uids?.includes(item._id)) || - [item.rid, item._id].includes(room._id), - ); - const usersFilter = (user: { _id: string }): boolean => - !localRooms.find((room) => room.t === 'd' && room.uids && room.uids?.length === 2 && room.uids.includes(user._id)); - const userMap = (user: { _id: string; name: string; @@ -104,15 +105,54 @@ export const useSearchItems = (filterText: string): UseQueryResult [item.name, item.fname].includes(name)); - return Array.from(new Set([...exact, ...localRooms, ...resultsFromServer])); + return resultsFromServer; }, staleTime: 60_000, - placeholderData: (previousData) => previousData ?? localRooms, + // Keep the previous server results visible while a new search is in flight. + placeholderData: (previousData) => previousData, }); + + // Merge reactively (outside the query) so local results render the instant the user types + // and server results fold in — deduped — once they arrive. + const items = useMemo(() => { + // Server results are keyed on the *debounced* term, so while the user is still typing + // (or via placeholderData) they may belong to a previous search. Drop the ones that no + // longer match the current text so stale, non-matching results aren't rendered. + const filterRegex = new RegExp(escapeRegExp(name), 'i'); + const matchesFilter = ({ name, fname }: { name?: string; fname?: string }) => + (name && filterRegex.test(name)) || (fname && filterRegex.test(fname)); + + // Single source of truth for local/server dedup: drop any server result already present as a + // local subscription, checked against the *current* localRooms. The query isn't keyed on + // localRooms (to avoid refetching on every subscription change), so subscriptions that load + // in after the fetch would otherwise render twice — once from localRooms, once from server. + const isLocalDuplicate = (item: { _id: string; t?: string; uids?: string[] }): boolean => + localRooms.some((room) => { + const sameRoom = [room.rid, room._id].includes(item._id); + const sameGroupDM = item.t === 'd' && !!item.uids && item.uids.length > 1 && item.uids.includes(room._id); + const sameDirectDM = item.t === 'd' && room.t === 'd' && !!room.uids && room.uids.length === 2 && room.uids.includes(item._id); + return sameRoom || sameGroupDM || sameDirectDM; + }); + + // When local subscriptions already fill the limit the server query is disabled, but React Query + // keeps the last results around — ignore them so a full local list isn't padded with stale rows. + const candidates = localRooms.length < LIMIT ? (serverResults ?? []) : []; + const fromServer = candidates.filter((item) => matchesFilter(item) && !isLocalDuplicate(item)); + const exact = fromServer.filter((item) => [item.name, item.fname].includes(name)); + return Array.from(new Set([...exact, ...localRooms, ...fromServer])) as SubscriptionWithRoom[]; + }, [serverResults, localRooms, name]); + + // `isFetching` is also true for silent background revalidations (after staleTime) of results we + // already show — only surface loading while there's no usable data for the current term yet, i.e. + // the very first fetch (serverResults undefined) or a new term still showing placeholder data. + const isLoading = isFetching && (isPlaceholderData || serverResults === undefined); + + return { items, isLoading }; }; diff --git a/apps/meteor/client/views/teams/ChannelDesertionTable/ChannelDesertionTableRow.tsx b/apps/meteor/client/views/teams/ChannelDesertionTable/ChannelDesertionTableRow.tsx index 6e412ac5fe2d1..cd51839374495 100644 --- a/apps/meteor/client/views/teams/ChannelDesertionTable/ChannelDesertionTableRow.tsx +++ b/apps/meteor/client/views/teams/ChannelDesertionTable/ChannelDesertionTableRow.tsx @@ -1,7 +1,8 @@ import type { IRoom, Serialized } from '@rocket.chat/core-typings'; -import { CheckBox, Icon, Margins } from '@rocket.chat/fuselage'; +import { Box, CheckBox, Icon, Margins } from '@rocket.chat/fuselage'; import { useStableCallback } from '@rocket.chat/fuselage-hooks'; import { GenericTableRow, GenericTableCell } from '@rocket.chat/ui-client'; +import { useId } from 'react'; import { useFormatDateAndTime } from '../../../hooks/useFormatDateAndTime'; @@ -15,17 +16,20 @@ type ChannelDesertionTableRowProps = { const ChannelDesertionTableRow = ({ room, onChange, selected, lastOwnerWarning }: ChannelDesertionTableRowProps) => { const { name, fname, ts, isLastOwner } = room; const formatDate = useFormatDateAndTime(); + const checkboxId = useId(); const handleChange = useStableCallback(() => onChange(room)); return ( - - - - {fname ?? name} - {isLastOwner && } - + + + + + {fname ?? name} + {isLastOwner && } + + {formatDate(ts)} diff --git a/apps/meteor/client/views/teams/contextualBar/members/RemoveUsersModal/RemoveUsersModal.tsx b/apps/meteor/client/views/teams/contextualBar/members/RemoveUsersModal/RemoveUsersModal.tsx index 79fe83084baf6..a1f766fef4933 100644 --- a/apps/meteor/client/views/teams/contextualBar/members/RemoveUsersModal/RemoveUsersModal.tsx +++ b/apps/meteor/client/views/teams/contextualBar/members/RemoveUsersModal/RemoveUsersModal.tsx @@ -18,7 +18,7 @@ const RemoveUsersModal = ({ teamId, userId, onClose, onCancel, onConfirm }: Remo const listRoomsOfUser = useEndpoint('GET', '/v1/teams.listRoomsOfUser'); const { isPending, data } = useQuery({ queryKey: teamsQueryKeys.roomsOfUser(teamId, userId), - queryFn: () => listRoomsOfUser({ teamId, userId, canUserDelete: 'true' }), + queryFn: () => listRoomsOfUser({ teamId, userId }), }); const getUserInfo = useEndpoint('GET', '/v1/users.info'); diff --git a/apps/meteor/tests/e2e/page-objects/fragments/flextabs/members-flextab.ts b/apps/meteor/tests/e2e/page-objects/fragments/flextabs/members-flextab.ts index 8d171076b2996..79c0be62583d4 100644 --- a/apps/meteor/tests/e2e/page-objects/fragments/flextabs/members-flextab.ts +++ b/apps/meteor/tests/e2e/page-objects/fragments/flextabs/members-flextab.ts @@ -22,6 +22,37 @@ export class ConfirmMuteModal extends Modal { } } +export class RemoveTeamMemberModal extends Modal { + constructor(page: Page) { + super(page.getByRole('dialog', { name: 'Removing Member' })); + } + + get description() { + return this.root.getByText('Select the channels you want the user to be removed from'); + } + + channel(channelName: string) { + return this.root.getByText(channelName); + } + + channelCheckbox(channelName: string) { + return this.root.getByRole('row', { name: channelName }).getByRole('checkbox'); + } + + async selectChannel(channelName: string) { + await this.channel(channelName).click(); + } + + private get btnContinue() { + return this.root.getByRole('button', { name: 'Continue' }); + } + + async continue() { + await this.btnContinue.click(); + await this.waitForDismissal(); + } +} + class AddUsersFlexTab extends FlexTab { readonly listbox: Listbox; @@ -47,6 +78,8 @@ export class MembersFlexTab extends FlexTab { readonly removeModal: ConfirmRemoveModal; + readonly removeTeamMemberModal: RemoveTeamMemberModal; + readonly menu: MenuMore; readonly confirmMuteModal: ConfirmMuteModal; @@ -58,6 +91,7 @@ export class MembersFlexTab extends FlexTab { constructor(page: Page) { super(page.getByRole('dialog', { name: 'Members' })); this.removeModal = new ConfirmRemoveModal(page.getByRole('dialog', { name: 'Confirmation', exact: true })); + this.removeTeamMemberModal = new RemoveTeamMemberModal(page); this.listbox = new Listbox(page); this.menu = new MenuMore(page); this.confirmMuteModal = new ConfirmMuteModal(page); diff --git a/apps/meteor/tests/e2e/team-management.spec.ts b/apps/meteor/tests/e2e/team-management.spec.ts index 08435079423e6..26487727e7e1f 100644 --- a/apps/meteor/tests/e2e/team-management.spec.ts +++ b/apps/meteor/tests/e2e/team-management.spec.ts @@ -3,7 +3,7 @@ import { faker } from '@faker-js/faker'; import { Users } from './fixtures/userStates'; import { HomeTeam } from './page-objects'; import { CreateNewTeamModal, CreateNewChannelModal } from './page-objects/fragments/modals'; -import { createTargetChannel } from './utils'; +import { createTargetChannel, deleteTeam, isChannelMember } from './utils'; import { expect, test } from './utils/test'; test.use({ storageState: Users.admin.state }); @@ -460,3 +460,61 @@ test.describe.serial('teams-management', () => { await expect(poHomeTeam.content.getSystemMessageByText(`converted #${targetTeam} to channel`)).toBeVisible(); }); }); + +test.describe('teams-management-remove-member-channel-selection', () => { + let poHomeTeam: HomeTeam; + + const targetTeamWithChannels = faker.string.uuid(); + let selectedChannel: string; + let unselectedChannel: string; + + test.beforeAll(async ({ api }) => { + const teamResponse = await api.post('/teams.create', { name: targetTeamWithChannels, type: 1, members: ['user1'] }); + await expect(teamResponse).toBeOK(); + const { team } = await teamResponse.json(); + + [selectedChannel, unselectedChannel] = await Promise.all([ + createTargetChannel(api, { members: ['user1'], extraData: { teamId: team._id } }), + createTargetChannel(api, { members: ['user1'], extraData: { teamId: team._id } }), + ]); + }); + + test.afterAll(async ({ api }) => { + await deleteTeam(api, targetTeamWithChannels); + }); + + test.beforeEach(async ({ page }) => { + poHomeTeam = new HomeTeam(page); + + await page.goto('/home'); + await poHomeTeam.waitForHome(); + }); + + test('should load the channel selection modal when removing a member that belongs to team channels', async ({ api }) => { + expect(await isChannelMember(api, selectedChannel, 'user1')).toBe(true); + expect(await isChannelMember(api, unselectedChannel, 'user1')).toBe(true); + + await poHomeTeam.navbar.openChat(targetTeamWithChannels); + await poHomeTeam.headerToolbar.openMoreOptions(); + await poHomeTeam.headerToolbar.openTeamMembers(); + await poHomeTeam.tabs.members.showAllUsers(); + await poHomeTeam.tabs.members.openMemberOptionMoreActions('user1'); + await poHomeTeam.tabs.members.getMenuItemAction('Remove from team').click(); + + await poHomeTeam.tabs.members.removeTeamMemberModal.waitForDisplay(); + await expect(poHomeTeam.tabs.members.removeTeamMemberModal.description).toBeVisible(); + await expect(poHomeTeam.tabs.members.removeTeamMemberModal.channel(selectedChannel)).toBeVisible(); + await expect(poHomeTeam.tabs.members.removeTeamMemberModal.channel(unselectedChannel)).toBeVisible(); + + await poHomeTeam.tabs.members.removeTeamMemberModal.selectChannel(selectedChannel); + await expect(poHomeTeam.tabs.members.removeTeamMemberModal.channelCheckbox(selectedChannel)).toBeChecked(); + await expect(poHomeTeam.tabs.members.removeTeamMemberModal.channelCheckbox(unselectedChannel)).not.toBeChecked(); + + await poHomeTeam.tabs.members.removeTeamMemberModal.continue(); + await poHomeTeam.tabs.members.confirmRemoveUser(); + + await expect(poHomeTeam.tabs.members.memberOption('user1')).not.toBeVisible(); + await expect.poll(() => isChannelMember(api, selectedChannel, 'user1')).toBe(false); + expect(await isChannelMember(api, unselectedChannel, 'user1')).toBe(true); + }); +}); diff --git a/apps/meteor/tests/e2e/utils/create-target-channel.ts b/apps/meteor/tests/e2e/utils/create-target-channel.ts index 105f25c360af6..25a590c768805 100644 --- a/apps/meteor/tests/e2e/utils/create-target-channel.ts +++ b/apps/meteor/tests/e2e/utils/create-target-channel.ts @@ -45,6 +45,12 @@ export async function deleteChannel(api: BaseTest['api'], roomName: string): Pro await api.post('/channels.delete', { roomName }); } +export async function isChannelMember(api: BaseTest['api'], roomName: string, username: string): Promise { + const response = await api.get(`/channels.members?roomName=${roomName}`); + const { members }: { members: { username: string }[] } = await response.json(); + return members.some((member) => member.username === username); +} + export async function deleteRoom(api: BaseTest['api'], roomId: string): Promise { await api.post('/rooms.delete', { roomId }); }