Improve distinct values retrieval

Author: d-gubert

apps/meteor/ee/server/apps/communication/endpoints/appLogsDistinctInstanceHandler.ts

@@ -1,9 +1,9 @@
-import { ajv } from '@rocket.chat/rest-typings/src/v1/Ajv';
+import { ajv } from '@rocket.chat/rest-typings';
 
 import type { AppsRestApi } from '../rest';
 
 // This might be a good candidate for a default validator function exported by @rocket.chat/rest-typings
-const errorResponse = ajv.compile({
+const errorResponse = ajv.compile<unknown>({
 	additionalProperties: false,
 	type: 'object',
 	properties: {
@@ -28,7 +28,7 @@ const errorResponse = ajv.compile({
 
 export const registerAppLogsDistinctInstanceHandler = ({ api, _orch }: AppsRestApi) =>
 	void api.get(
-		':id/logs/instanceIds',
+		':id/logs/distinctValues',
 		{
 			authRequired: true,
 			permissionsRequired: ['manage-apps'],
@@ -45,9 +45,16 @@ export const registerAppLogsDistinctInstanceHandler = ({ api, _orch }: AppsRestA
 				}),
 				401: errorResponse,
 				403: errorResponse,
+				404: errorResponse,
 			},
 		},
 		async function action() {
+			const app = await _orch.getManager()?.getOneById(this.urlParams.id);
+
+			if (!app) {
+				return api.notFound(`No app found with id: ${this.urlParams.id}`);
+			}
+
 			const result = await _orch.getLogStorage().distinctValues(this.urlParams.id);
 
 			return api.success(result);

…-end/apps/app-logs-distinct-instances.ts …-to-end/apps/app-logs-distinct-values.tsapps/meteor/tests/end-to-end/apps/app-logs-distinct-instances.ts renamed to apps/meteor/tests/end-to-end/apps/app-logs-distinct-values.ts apps/meteor/tests/end-to-end/apps/app-logs-distinct-instances.ts renamed to apps/meteor/tests/end-to-end/apps/app-logs-distinct-values.ts

@@ -7,38 +7,46 @@ import { installTestApp, cleanupApps } from '../../data/apps/helper';
 import { updatePermission } from '../../data/permissions.helper';
 import { IS_EE } from '../../e2e/config/constants';
 
-(IS_EE ? describe : describe.skip)('Apps - Logs Distinct Instances', () => {
+(IS_EE ? describe : describe.skip)('Apps - Logs Distinct Values', () => {
+	let appId: string;
+
 	before((done) => getCredentials(done));
 
 	before(async () => {
 		await cleanupApps();
-		await installTestApp();
+		const app = await installTestApp();
+		appId = app.id;
 	});
 
 	after(() => Promise.all([cleanupApps(), updatePermission('manage-apps', ['admin'])]));
 
-	it('should return distinct instance IDs successfully', (done) => {
+	it('should return distinct values successfully', (done) => {
 		void request
-			.get(apps('/logs/instanceIds'))
+			.get(apps(`/${appId}/logs/distinctValues`))
 			.set(credentials)
 			.expect(200)
 			.expect('Content-Type', 'application/json')
 			.expect((res) => {
 				expect(res.body).to.have.a.property('success', true);
 				expect(res.body).to.have.a.property('instanceIds').that.is.an('array');
+				expect(res.body).to.have.a.property('methods').that.is.an('array');
 
-				const { instanceIds } = res.body;
-				const uniqueInstanceIds = [...new Set(instanceIds)];
+				const { instanceIds, methods } = res.body;
+				const uniqueInstanceIds = new Set(instanceIds);
+				const uniqueMethods = new Set(methods);
 
 				// All instance IDs should be unique
-				expect(instanceIds).to.have.lengthOf(uniqueInstanceIds.length);
+				expect(instanceIds).to.have.lengthOf(uniqueInstanceIds.size);
+
+				// All methods should be unique
+				expect(methods).to.have.lengthOf(uniqueMethods.size);
 			})
 			.end(done);
 	});
 
 	it('should require authentication', (done) => {
 		void request
-			.get(apps('/logs/instanceIds'))
+			.get(apps(`/${appId}/logs/distinctValues`))
 			.expect(401)
 			.expect('Content-Type', 'application/json')
 			.expect((res) => {
@@ -52,7 +60,7 @@ import { IS_EE } from '../../e2e/config/constants';
 		void updatePermission('manage-apps', []).then(
 			() =>
 				void request
-					.get(apps('/logs/instanceIds'))
+					.get(apps(`/${appId}/logs/distinctValues`))
 					.set(credentials)
 					.expect(403)
 					.expect('Content-Type', 'application/json')
@@ -64,19 +72,36 @@ import { IS_EE } from '../../e2e/config/constants';
 		);
 	});
 
-	it('should return empty array when no instances exist', (done) => {
-		// Clean up all apps first to ensure no instances
-		void cleanupApps().then(() => {
+	it('should return 404 for non-existent app', (done) => {
+		void request
+			.get(apps('/non-existent-app-id/logs/distinctValues'))
+			.set(credentials)
+			.expect(404)
+			.expect('Content-Type', 'application/json')
+			.expect((res) => {
+				expect(res.body).to.have.a.property('success', false);
+				expect(res.body).to.have.a.property('error');
+			})
+			.end(done);
+	});
+
+	it('should return empty arrays when no logs exist', (done) => {
+		// Clean up all apps first to ensure no logs
+		void cleanupApps().then(async () => {
+			// Install a fresh app
+			const app = await installTestApp();
 			void request
-				.get(apps('/logs/instanceIds'))
+				.get(apps(`/${app.id}/logs/distinctValues`))
 				.set(credentials)
 				.expect(200)
 				.expect('Content-Type', 'application/json')
 				.expect((res) => {
 					expect(res.body).to.have.a.property('success', true);
 					expect(res.body).to.have.a.property('instanceIds').that.is.an('array');
-					// Could be empty or have instances from other tests
+					expect(res.body).to.have.a.property('methods').that.is.an('array');
+					// Could be empty or have values from other tests
 					expect(res.body.instanceIds).to.satisfy((arr: any[]) => Array.isArray(arr));
+					expect(res.body.methods).to.satisfy((arr: any[]) => Array.isArray(arr));
 				})
 				.end(done);
 		});