feat!: shrink payload of the startImport endpoint (#33630)

.changeset/two-emus-wash.md

@@ -0,0 +1,7 @@
+---
+'@rocket.chat/core-typings': major
+'@rocket.chat/rest-typings': major
+'@rocket.chat/meteor': major
+---
+
+Changes the payload of the startImport endpoint to decrease the amount of data it requires

apps/meteor/app/importer-pending-avatars/server/PendingAvatarImporter.ts

@@ -1,6 +1,7 @@
+import type { IImporterShortSelection } from '@rocket.chat/core-typings';
 import { Users } from '@rocket.chat/models';
 
-import { Importer, ProgressStep, Selection } from '../../importer/server';
+import { Importer, ProgressStep } from '../../importer/server';
 import type { ImporterProgress } from '../../importer/server/classes/ImporterProgress';
 import { setAvatarFromServiceWithValidation } from '../../lib/server/functions/setUserAvatar';
 
@@ -17,21 +18,23 @@ export class PendingAvatarImporter extends Importer {
 			return 0;
 		}
 
-		await this.updateRecord({ 'count.messages': fileCount, 'messagesstatus': null });
-		await this.addCountToTotal(fileCount);
-
-		const fileData = new Selection(this.info.name, [], [], fileCount);
-		await this.updateRecord({ fileData });
+		this.progress.count.total += fileCount;
+		await this.updateRecord({
+			'count.messages': fileCount,
+			'count.total': fileCount,
+			'messagesstatus': null,
+			'status': ProgressStep.IMPORTING_FILES,
+		});
+		this.reportProgress();
 
-		await super.updateProgress(ProgressStep.IMPORTING_FILES);
 		setImmediate(() => {
-			void this.startImport(fileData);
+			void this.startImport({});
 		});
 
 		return fileCount;
 	}
 
-	async startImport(importSelection: Selection): Promise<ImporterProgress> {
+	async startImport(importSelection: IImporterShortSelection): Promise<ImporterProgress> {
 		const pendingFileUserList = Users.findAllUsersWithPendingAvatar();
 		try {
 			for await (const user of pendingFileUserList) {

apps/meteor/app/importer-pending-files/server/PendingFileImporter.ts

@@ -2,12 +2,12 @@ import http from 'http';
 import https from 'https';
 
 import { api } from '@rocket.chat/core-services';
-import type { IImport, MessageAttachment, IUpload } from '@rocket.chat/core-typings';
+import type { IImport, MessageAttachment, IUpload, IImporterShortSelection } from '@rocket.chat/core-typings';
 import { Messages } from '@rocket.chat/models';
 import { Random } from '@rocket.chat/random';
 
 import { FileUpload } from '../../file-upload/server';
-import { Importer, ProgressStep, Selection } from '../../importer/server';
+import { Importer, ProgressStep } from '../../importer/server';
 import type { ConverterOptions } from '../../importer/server/classes/ImportDataConverter';
 import type { ImporterProgress } from '../../importer/server/classes/ImporterProgress';
 import type { ImporterInfo } from '../../importer/server/definitions/ImporterInfo';
@@ -27,21 +27,23 @@ export class PendingFileImporter extends Importer {
 			return 0;
 		}
 
-		await this.updateRecord({ 'count.messages': fileCount, 'messagesstatus': null });
-		await this.addCountToTotal(fileCount);
-
-		const fileData = new Selection(this.info.name, [], [], fileCount);
-		await this.updateRecord({ fileData });
+		this.progress.count.total += fileCount;
+		await this.updateRecord({
+			'count.messages': fileCount,
+			'count.total': fileCount,
+			'messagesstatus': null,
+			'status': ProgressStep.IMPORTING_FILES,
+		});
+		this.reportProgress();
 
-		await super.updateProgress(ProgressStep.IMPORTING_FILES);
 		setImmediate(() => {
-			void this.startImport(fileData);
+			void this.startImport({});
 		});
 
 		return fileCount;
 	}
 
-	async startImport(importSelection: Selection): Promise<ImporterProgress> {
+	async startImport(importSelection: IImporterShortSelection): Promise<ImporterProgress> {
 		const downloadedFileIds: string[] = [];
 		const maxFileCount = 10;
 		const maxFileSize = 1024 * 1024 * 500;

apps/meteor/app/importer/server/classes/Importer.ts

@@ -1,12 +1,18 @@
 import { api } from '@rocket.chat/core-services';
-import type { IImport, IImportRecord, IImportChannel, IImportUser, IImportProgress } from '@rocket.chat/core-typings';
+import type {
+	IImport,
+	IImportRecord,
+	IImportChannel,
+	IImportUser,
+	IImportProgress,
+	IImporterShortSelection,
+} from '@rocket.chat/core-typings';
 import { Logger } from '@rocket.chat/logger';
 import { Settings, ImportData, Imports } from '@rocket.chat/models';
 import AdmZip from 'adm-zip';
 import type { MatchKeysAndValues, MongoServerError } from 'mongodb';
 
 import { Selection, SelectionChannel, SelectionUser } from '..';
-import { callbacks } from '../../../../lib/callbacks';
 import { notifyOnSettingChangedById } from '../../../lib/server/lib/notifyListener';
 import { t } from '../../../utils/lib/i18n';
 import { ProgressStep, ImportPreparingStartedStates } from '../../lib/ImporterProgressStep';
@@ -91,27 +97,10 @@ export class Importer {
 	 * doesn't end up with a "locked" UI while Meteor waits for a response.
 	 * The returned object should be the progress.
 	 *
-	 * @param {Selection} importSelection The selection data.
+	 * @param {IImporterShortSelection} importSelection The selection data.
 	 * @returns {ImporterProgress} The progress record of the import.
 	 */
-	async startImport(importSelection: Selection, startedByUserId: string): Promise<ImporterProgress> {
-		if (!(importSelection instanceof Selection)) {
-			throw new Error(`Invalid Selection data provided to the ${this.info.name} importer.`);
-		} else if (importSelection.users === undefined) {
-			throw new Error(`Users in the selected data wasn't found, it must but at least an empty array for the ${this.info.name} importer.`);
-		} else if (importSelection.channels === undefined) {
-			throw new Error(
-				`Channels in the selected data wasn't found, it must but at least an empty array for the ${this.info.name} importer.`,
-			);
-		}
-		if (!startedByUserId) {
-			throw new Error('You must be logged in to do this.');
-		}
-
-		if (!startedByUserId) {
-			throw new Error('You must be logged in to do this.');
-		}
-
+	async startImport(importSelection: IImporterShortSelection, startedByUserId: string): Promise<ImporterProgress> {
 		await this.updateProgress(ProgressStep.IMPORTING_STARTED);
 		this.reloadCount();
 		const started = Date.now();
@@ -124,37 +113,29 @@ export class Importer {
 
 			switch (type) {
 				case 'channel': {
-					if (!importSelection.channels) {
+					if (importSelection.channels?.all) {
 						return true;
 					}
+					if (!importSelection.channels?.list?.length) {
+						return false;
+					}
 
 					const channelData = data as IImportChannel;
 
 					const id = channelData.t === 'd' ? '__directMessages__' : channelData.importIds[0];
-					for (const channel of importSelection.channels) {
-						if (channel.channel_id === id) {
-							return channel.do_import;
-						}
-					}
-
-					return false;
+					return importSelection.channels.list?.includes(id);
 				}
 				case 'user': {
-					// #TODO: Replace this workaround
-					if (importSelection.users.length === 0 && this.info.key === 'api') {
+					if (importSelection.users?.all) {
 						return true;
 					}
+					if (!importSelection.users?.list?.length) {
+						return false;
+					}
 
 					const userData = data as IImportUser;
-
 					const id = userData.importIds[0];
-					for (const user of importSelection.users) {
-						if (user.user_id === id) {
-							return user.do_import;
-						}
-					}
-
-					return false;
+					return importSelection.users.list.includes(id);
 				}
 			}
 
@@ -198,8 +179,6 @@ export class Importer {
 				await this.applySettingValues({});
 
 				await this.updateProgress(ProgressStep.IMPORTING_USERS);
-				const usersToImport = importSelection.users.filter((user) => user.do_import);
-				await callbacks.run('beforeUserImport', { userCount: usersToImport.length });
 				await this.converter.convertUsers({ beforeImportFn, afterImportFn, onErrorFn, afterBatchFn });
 
 				await this.updateProgress(ProgressStep.IMPORTING_CHANNELS);

apps/meteor/app/importer/server/methods/startImport.ts

@@ -1,10 +1,10 @@
 import type { IUser } from '@rocket.chat/core-typings';
 import type { ServerMethods } from '@rocket.chat/ddp-client';
 import { Imports } from '@rocket.chat/models';
-import type { StartImportParamsPOST } from '@rocket.chat/rest-typings';
+import { isStartImportParamsPOST, type StartImportParamsPOST } from '@rocket.chat/rest-typings';
 import { Meteor } from 'meteor/meteor';
 
-import { Importers, Selection, SelectionChannel, SelectionUser } from '..';
+import { Importers } from '..';
 import { hasPermissionAsync } from '../../../authorization/server/functions/hasPermission';
 
 export const executeStartImport = async ({ input }: StartImportParamsPOST, startedByUserId: IUser['_id']) => {
@@ -21,15 +21,7 @@ export const executeStartImport = async ({ input }: StartImportParamsPOST, start
 
 	const instance = new importer.importer(importer, operation); // eslint-disable-line new-cap
 
-	const usersSelection = input.users.map(
-		(user) => new SelectionUser(user.user_id, user.username, user.email, user.is_deleted, user.is_bot, user.do_import),
-	);
-	const channelsSelection = input.channels.map(
-		(channel) =>
-			new SelectionChannel(channel.channel_id, channel.name, channel.is_archived, channel.do_import, channel.is_private, channel.is_direct),
-	);
-	const selection = new Selection(importer.name, usersSelection, channelsSelection, 0);
-	await instance.startImport(selection, startedByUserId);
+	await instance.startImport(input, startedByUserId);
 };
 
 declare module '@rocket.chat/ddp-client' {
@@ -41,6 +33,10 @@ declare module '@rocket.chat/ddp-client' {
 
 Meteor.methods<ServerMethods>({
 	async startImport({ input }: StartImportParamsPOST) {
+		if (!input || typeof input !== 'object' || !isStartImportParamsPOST({ input })) {
+			throw new Meteor.Error(`Invalid Selection data provided to the importer.`);
+		}
+
 		const userId = Meteor.userId();
 		// Takes name and object with users / channels selected to import
 		if (!userId) {

apps/meteor/client/views/admin/import/PrepareImportPage.tsx

@@ -151,10 +151,19 @@ function PrepareImportPage() {
 		setImporting(true);
 
 		try {
+			const usersToImport = users.filter(({ do_import }) => do_import).map(({ user_id }) => user_id);
+			const channelsToImport = channels.filter(({ do_import }) => do_import).map(({ channel_id }) => channel_id);
+
 			await startImport({
 				input: {
-					users: users.map((user) => ({ is_bot: false, is_email_taken: false, ...user })),
-					channels: channels.map((channel) => ({ is_private: false, is_direct: false, ...channel })),
+					users: {
+						all: users.length > 0 && usersToImport.length === users.length,
+						list: (usersToImport.length !== users.length && usersToImport) || undefined,
+					},
+					channels: {
+						all: channels.length > 0 && channelsToImport.length === channels.length,
+						list: (channelsToImport.length !== channels.length && channelsToImport) || undefined,
+					},
 				},
 			});
 			router.navigate('/admin/import/progress');

apps/meteor/lib/callbacks.ts

@@ -99,7 +99,6 @@ interface EventLikeCallbackSignatures {
 	'beforeSaveUser': ({ user, oldUser }: { user: IUser; oldUser?: IUser }) => void;
 	'afterSaveUser': ({ user, oldUser }: { user: IUser; oldUser?: IUser | null }) => void;
 	'livechat.afterTagRemoved': (tag: ILivechatTagRecord) => void;
-	'beforeUserImport': (data: { userCount: number }) => void;
 	'afterUserImport': (data: { inserted: IUser['_id'][]; updated: IUser['_id']; skipped: number; failed: number }) => void;
 }
 

apps/meteor/server/services/import/service.ts

@@ -5,7 +5,6 @@ import { Imports, ImportData } from '@rocket.chat/models';
 import { ObjectId } from 'mongodb';
 
 import { Importers } from '../../../app/importer/server';
-import { ImporterSelection } from '../../../app/importer/server/classes/ImporterSelection';
 import { settings } from '../../../app/settings/server';
 import { validateRoleList } from '../../lib/roles/validateRoleList';
 import { getNewUserRoles } from '../user/lib/getNewUserRoles';
@@ -175,7 +174,6 @@ export class ImportService extends ServiceClassInternal implements IImportServic
 			skipExistingUsers: true,
 		});
 
-		const selection = new ImporterSelection(importer.name, [], [], 0);
-		await instance.startImport(selection, userId);
+		await instance.startImport({ users: { all: true } }, userId);
 	}
 }

apps/meteor/tests/e2e/fixtures/files/csv_import_users.csv

@@ -1,2 +1,3 @@
 billy.bob, billy.bob@example.com, Billy Bob Jr.
 billy.joe, billy.joe@example.com, Billy Joe Jr.
+billy.billy, billy.billy@example.com, Billy Billy Jr.

apps/meteor/tests/e2e/imports.spec.ts

@@ -115,6 +115,8 @@ test.describe.serial('imports', () => {
 		await poAdmin.inputFile.setInputFiles(zipCsvImportDir);
 		await poAdmin.btnImport.click();
 
+		await poAdmin.findFileCheckboxByUsername('billy.billy').click();
+
 		await poAdmin.btnStartImport.click();
 
 		await expect(poAdmin.importStatusTableFirstRowCell).toBeVisible({
@@ -125,8 +127,12 @@ test.describe.serial('imports', () => {
 	test('expect all imported users to be actually listed as users', async ({ page }) => {
 		await page.goto('/admin/users');
 
-		for (const user of rowUserName) {
-			expect(page.locator(`tbody tr td:first-child >> text="${user}"`));
+		for await (const user of rowUserName) {
+			if (user === 'billy.billy') {
+				await expect(page.locator(`tbody tr td:first-child >> text="${user}"`)).not.toBeVisible();
+			} else {
+				expect(page.locator(`tbody tr td:first-child >> text="${user}"`));
+			}
 		}
 	});
 

apps/meteor/tests/e2e/page-objects/admin.ts

@@ -298,4 +298,12 @@ export class Admin {
 	async adminSectionButton(href: AdminSectionsHref): Promise<Locator> {
 		return this.page.locator(`a[href="${href}"]`);
 	}
+
+	findFileRowByUsername(username: string) {
+		return this.page.locator('tr', { has: this.page.getByRole('cell', { name: username }) });
+	}
+
+	findFileCheckboxByUsername(username: string) {
+		return this.findFileRowByUsername(username).locator('label', { has: this.page.getByRole('checkbox') });
+	}
 }

packages/core-typings/src/import/IImporterShortSelection.ts

@@ -0,0 +1,9 @@
+export interface IImporterShortSelectionItem {
+	all?: boolean;
+	list?: string[];
+}
+
+export interface IImporterShortSelection {
+	users?: IImporterShortSelectionItem;
+	channels?: IImporterShortSelectionItem;
+}

packages/core-typings/src/import/index.ts

@@ -9,4 +9,5 @@ export * from './IImportProgress';
 export * from './IImporterSelection';
 export * from './IImporterSelectionUser';
 export * from './IImporterSelectionChannel';
+export * from './IImporterShortSelection';
 export * from './ImportState';