fix: "Discussions" filter is prioritized in admin "Rooms" page (Rocke…

…tChat#28426) Co-authored-by: gabriellsh <[email protected]> Co-authored-by: matheusbsilva137 <[email protected]> Co-authored-by: Matheus Barbosa Silva <[email protected]>
sampaiodiego · Jun 13, 2023 · 16dca46 · 16dca46
1 parent 7967d46
commit 16dca46
Show file tree

Hide file tree

Showing 8 changed files with 117 additions and 115 deletions.
diff --git a/.changeset/wet-tomatoes-happen.md b/.changeset/wet-tomatoes-happen.md
@@ -0,0 +1,6 @@
+---
+"@rocket.chat/meteor": patch
+"@rocket.chat/model-typings": patch
+---
+
+fix: "Discussions" filter is prioritized in admin "Rooms" page
diff --git a/apps/meteor/app/api/server/lib/rooms.ts b/apps/meteor/app/api/server/lib/rooms.ts
@@ -27,7 +27,6 @@ export async function findAdminRooms({
 	const name = filter?.trim();
 	const discussion = types?.includes('discussions');
 	const includeTeams = types?.includes('teams');
-	const showOnlyTeams = types.length === 1 && types.includes('teams');
 	const typesToRemove = ['discussions', 'teams'];
 	const showTypes = Array.isArray(types) ? types.filter((type): type is RoomType => !typesToRemove.includes(type)) : [];
 	const options: FindOptions<IRoom> = {
@@ -36,14 +35,7 @@ export async function findAdminRooms({
 		limit: count,
 	};
 
-	let result;
-	if (name && showTypes.length) {
-		result = Rooms.findByNameOrFnameContainingAndTypes(name, showTypes, discussion, includeTeams, showOnlyTeams, options);
-	} else if (showTypes.length) {
-		result = Rooms.findByTypes(showTypes, discussion, includeTeams, showOnlyTeams, options);
-	} else {
-		result = Rooms.findByNameOrFnameContaining(name, discussion, includeTeams, showOnlyTeams, options);
-	}
+	const result = Rooms.findByNameOrFnameContainingAndTypes(name, showTypes, discussion, includeTeams, options);
 
 	const { cursor, totalCount } = result;
 

diff --git a/apps/meteor/app/api/server/v1/rooms.ts b/apps/meteor/app/api/server/v1/rooms.ts
@@ -398,7 +398,7 @@ API.v1.addRoute(
 				await findAdminRooms({
 					uid: this.userId,
 					filter: filter || '',
-					types: types || [],
+					types: (types && !Array.isArray(types) ? [types] : types) ?? [],
 					pagination: {
 						offset,
 						count,

diff --git a/apps/meteor/client/views/admin/rooms/FilterByTypeAndText.tsx b/apps/meteor/client/views/admin/rooms/FilterByTypeAndText.tsx
@@ -4,17 +4,17 @@ import { useTranslation } from '@rocket.chat/ui-contexts';
 import type { ReactElement, Dispatch, SetStateAction } from 'react';
 import React, { useCallback, useState, useEffect } from 'react';
 
-const DEFAULT_TYPES = ['d', 'p', 'c', 'teams'];
+const DEFAULT_TYPES = ['d', 'p', 'c', 'l', 'discussions', 'teams'];
 
 const FilterByTypeAndText = ({ setFilter, ...props }: { setFilter?: Dispatch<SetStateAction<any>> }): ReactElement => {
 	const [text, setText] = useState('');
 	const [types, setTypes] = useState({
-		d: false,
-		c: false,
-		p: false,
-		l: false,
-		discussions: false,
-		teams: false,
+		d: true,
+		c: true,
+		p: true,
+		l: true,
+		discussions: true,
+		teams: true,
 	});
 
 	const t = useTranslation();

diff --git a/apps/meteor/client/views/admin/rooms/RoomsTable.tsx b/apps/meteor/client/views/admin/rooms/RoomsTable.tsx
@@ -29,7 +29,7 @@ type RoomFilters = {
 	text: string;
 };
 
-const DEFAULT_TYPES = ['d', 'p', 'c', 'teams'];
+const DEFAULT_TYPES = ['d', 'p', 'c', 'l', 'discussions', 'teams'];
 
 const roomTypeI18nMap = {
 	l: 'Omnichannel',

diff --git a/apps/meteor/server/models/raw/Rooms.ts b/apps/meteor/server/models/raw/Rooms.ts
@@ -164,26 +164,11 @@ export class RoomsRaw extends BaseRaw<IRoom> implements IRoomsModel {
 		types: Array<IRoom['t']>,
 		discussion = false,
 		teams = false,
-		showOnlyTeams = false,
 		options: FindOptions<IRoom> = {},
 	): FindPaginated<FindCursor<IRoom>> {
 		const nameRegex = new RegExp(escapeRegExp(name).trim(), 'i');
 
-		const onlyTeamsQuery: Filter<IRoom> = showOnlyTeams ? { teamMain: { $exists: true } } : {};
-
-		const teamCondition: Filter<IRoom> = teams
-			? {}
-			: {
-					teamMain: {
-						$exists: false,
-					},
-			  };
-
-		const query: Filter<IRoom> = {
-			t: {
-				$in: types,
-			},
-			prid: { $exists: discussion },
+		const nameCondition: Filter<IRoom> = {
 			$or: [
 				{ name: nameRegex, federated: { $ne: true } },
 				{ fname: nameRegex },
@@ -192,71 +177,27 @@ export class RoomsRaw extends BaseRaw<IRoom> implements IRoomsModel {
 					usernames: nameRegex,
 				},
 			],
-			...teamCondition,
-			...onlyTeamsQuery,
 		};
-		return this.findPaginated(query, options);
-	}
-
-	findByTypes(
-		types: Array<IRoom['t']>,
-		discussion = false,
-		teams = false,
-		onlyTeams = false,
-		options: FindOptions<IRoom> = {},
-	): FindPaginated<FindCursor<IRoom>> {
-		const teamCondition = teams
-			? {}
-			: {
-					teamMain: {
-						$exists: false,
-					},
-			  };
-
-		const onlyTeamsCondition = onlyTeams ? { teamMain: { $exists: true } } : {};
 
 		const query: Filter<IRoom> = {
-			t: {
-				$in: types,
-			},
-			prid: { $exists: discussion },
-			...teamCondition,
-			...onlyTeamsCondition,
-		};
-		return this.findPaginated(query, options);
-	}
-
-	findByNameOrFnameContaining(
-		name: NonNullable<IRoom['name']>,
-		discussion = false,
-		teams = false,
-		onlyTeams = false,
-		options: FindOptions<IRoom> = {},
-	): FindPaginated<FindCursor<IRoom>> {
-		const nameRegex = new RegExp(escapeRegExp(name).trim(), 'i');
-
-		const teamCondition = teams
-			? {}
-			: {
-					teamMain: {
-						$exists: false,
-					},
-			  };
-
-		const onlyTeamsCondition = onlyTeams ? { $and: [{ teamMain: { $exists: true } }, { teamMain: true }] } : {};
-
-		const query: Filter<IRoom> = {
-			prid: { $exists: discussion },
-			$or: [
-				{ name: nameRegex, federated: { $ne: true } },
-				{ fname: nameRegex },
-				{
-					t: 'd',
-					usernames: nameRegex,
-				},
+			$and: [
+				name ? nameCondition : {},
+				(types && types.length) || discussion || teams
+					? {
+							$or: [
+								{
+									t: {
+										$in: types,
+									},
+								},
+								...(discussion ? [{ prid: { $exists: true } }] : []),
+								...(teams ? [{ teamMain: { $exists: true } }] : []),
+							],
+					  }
+					: {},
 			],
-			...teamCondition,
-			...onlyTeamsCondition,
+			...(!discussion ? { prid: { $exists: false } } : {}),
+			...(!teams ? { teamMain: { $exists: false } } : {}),
 		};
 
 		return this.findPaginated(query, options);

diff --git a/apps/meteor/tests/end-to-end/api/09-rooms.js b/apps/meteor/tests/end-to-end/api/09-rooms.js
@@ -1306,10 +1306,23 @@ describe('[Rooms]', function () {
 		const suffix = `test-${Date.now()}`;
 		const fnameRoom = `Ελληνικά-${suffix}`;
 		const nameRoom = `Ellinika-${suffix}`;
+		const discussionRoomName = `${nameRoom}-discussion`;
+
+		let testGroup;
 
 		before((done) => {
 			updateSetting('UI_Allow_room_names_with_special_chars', true).then(() => {
-				createRoom({ type: 'p', name: fnameRoom }).end(done);
+				createRoom({ type: 'p', name: fnameRoom }).end((err, res) => {
+					testGroup = res.body.group;
+					request
+						.post(api('rooms.createDiscussion'))
+						.set(credentials)
+						.send({
+							prid: testGroup._id,
+							t_name: discussionRoomName,
+						})
+						.end(done);
+				});
 			});
 		});
 
@@ -1400,6 +1413,73 @@ describe('[Rooms]', function () {
 					.end(done);
 			});
 		});
+		it('should filter by only rooms types', (done) => {
+			request
+				.get(api('rooms.adminRooms'))
+				.set(credentials)
+				.query({
+					types: ['p'],
+				})
+				.expect(200)
+				.expect((res) => {
+					expect(res.body).to.have.property('success', true);
+					expect(res.body).to.have.property('rooms').and.to.be.an('array');
+					expect(res.body.rooms).to.have.lengthOf.at.least(1);
+					expect(res.body.rooms[0].t).to.be.equal('p');
+					expect(res.body.rooms.find((room) => room.name === nameRoom)).to.exist;
+					expect(res.body.rooms.find((room) => room.name === discussionRoomName)).to.not.exist;
+				})
+				.end(done);
+		});
+		it('should filter by only name', (done) => {
+			request
+				.get(api('rooms.adminRooms'))
+				.set(credentials)
+				.query({
+					filter: nameRoom,
+				})
+				.expect(200)
+				.expect((res) => {
+					expect(res.body).to.have.property('success', true);
+					expect(res.body).to.have.property('rooms').and.to.be.an('array');
+					expect(res.body.rooms).to.have.lengthOf(1);
+					expect(res.body.rooms[0].name).to.be.equal(nameRoom);
+				})
+				.end(done);
+		});
+		it('should filter by type and name at the same query', (done) => {
+			request
+				.get(api('rooms.adminRooms'))
+				.set(credentials)
+				.query({
+					filter: nameRoom,
+					types: ['p'],
+				})
+				.expect(200)
+				.expect((res) => {
+					expect(res.body).to.have.property('success', true);
+					expect(res.body).to.have.property('rooms').and.to.be.an('array');
+					expect(res.body.rooms).to.have.lengthOf(1);
+					expect(res.body.rooms[0].name).to.be.equal(nameRoom);
+				})
+				.end(done);
+		});
+		it('should return an empty array when filter by wrong type and correct room name', (done) => {
+			request
+				.get(api('rooms.adminRooms'))
+				.set(credentials)
+				.query({
+					filter: nameRoom,
+					types: ['c'],
+				})
+				.expect(200)
+				.expect((res) => {
+					expect(res.body).to.have.property('success', true);
+					expect(res.body).to.have.property('rooms').and.to.be.an('array');
+					expect(res.body.rooms).to.have.lengthOf(0);
+				})
+				.end(done);
+		});
 	});
 
 	describe('update group dms name', () => {

diff --git a/packages/model-typings/src/models/IRoomsModel.ts b/packages/model-typings/src/models/IRoomsModel.ts
@@ -32,25 +32,8 @@ export interface IRoomsModel extends IBaseModel<IRoom> {
 	findByNameOrFnameContainingAndTypes(
 		name: NonNullable<IRoom['name']>,
 		types: Array<IRoom['t']>,
-		discussion: boolean,
-		teams: boolean,
-		showOnlyTeams: boolean,
-		options?: FindOptions<IRoom>,
-	): FindPaginated<FindCursor<IRoom>>;
-
-	findByTypes(
-		types: Array<IRoom['t']>,
-		discussion: boolean,
-		teams: boolean,
-		onlyTeams: boolean,
-		options?: FindOptions<IRoom>,
-	): FindPaginated<FindCursor<IRoom>>;
-
-	findByNameOrFnameContaining(
-		name: NonNullable<IRoom['name']>,
-		discussion: boolean,
-		teams: boolean,
-		onlyTeams: boolean,
+		discussion?: boolean,
+		teams?: boolean,
 		options?: FindOptions<IRoom>,
 	): FindPaginated<FindCursor<IRoom>>;