Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Room import doesn't honor the specified owner #31803

Merged
merged 15 commits into from
Mar 4, 2024
Merged
Show file tree
Hide file tree
Changes from 12 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/funny-cooks-sneeze.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@rocket.chat/meteor": patch
---

Fixed room owner specified on room import not being inserted as a room member or owner.
Original file line number Diff line number Diff line change
Expand Up @@ -1020,13 +1020,13 @@ export class ImportDataConverter {
return;
}
if (roomData.t === 'p') {
const user = await Users.findOneById(startedByUserId);
const user = await Users.findOneById(creatorId);
if (!user) {
throw new Error('importer-channel-invalid-creator');
}
roomInfo = await createPrivateGroupMethod(user, roomData.name, members, false, {}, {}, true);
roomInfo = await createPrivateGroupMethod(user, roomData.name, members, false, {}, {});
} else {
roomInfo = await createChannelMethod(startedByUserId, roomData.name, members, false, {}, {}, true);
roomInfo = await createChannelMethod(creatorId, roomData.name, members, false, {}, {});
}
}

Expand Down
2 changes: 1 addition & 1 deletion apps/meteor/client/views/admin/rooms/EditRoom.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -182,7 +182,7 @@ const EditRoom = ({ room, onChange, onDelete }: EditRoomProps) => {
<Field>
<FieldLabel htmlFor={ownerField}>{t('Owner')}</FieldLabel>
<FieldRow>
<TextInput id={ownerField} readOnly value={room.u?.username} />
<TextInput id={ownerField} name='roomOwner' readOnly value={room.u?.username} />
</FieldRow>
</Field>
)}
Expand Down
Binary file not shown.
4 changes: 4 additions & 0 deletions apps/meteor/tests/e2e/fixtures/files/csv_import_rooms.csv
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
"imported-csv-group","billy.bob","private",""
"imported-csv-channel","billy.bob","public",""
"imported-csv-with-members-group","billy.bob","private","billy.bob;billy.joe"
"imported-csv-with-members-channel","billy.bob","public","billy.bob;billy.joe"
2 changes: 2 additions & 0 deletions apps/meteor/tests/e2e/fixtures/files/csv_import_users.csv
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
billy.bob, [email protected], Billy Bob Jr.
billy.joe, [email protected], Billy Joe Jr.
2 changes: 2 additions & 0 deletions apps/meteor/tests/e2e/fixtures/files/dm_messages.csv
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
"billy.joe","billy.bob","1479162481336","this is a test message"
"billy.bob","billy.joe","1479162481654","this is another message, a test message"
111 changes: 108 additions & 3 deletions apps/meteor/tests/e2e/imports.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,20 +9,66 @@ import { test, expect } from './utils/test';

test.use({ storageState: Users.admin.state });

type csvRoomSpec = {
name: string;
ownerUsername: string;
visibility: 'public' | 'private';
members: string;
};

const rowUserName: string[] = [];
const csvImportedUsernames: string[] = [];
const dmMessages: string[] = [];
const importedRooms: csvRoomSpec[] = [];
const slackCsvDir = path.resolve(__dirname, 'fixtures', 'files', 'slack_export_users.csv');
const zipCsvImportDir = path.resolve(__dirname, 'fixtures', 'files', 'csv_import.zip');

// These files have the same content from users.csv, channels.csv and messages1.csv from the zip file
// They have been extracted just so that we don't need to do that on the fly
const usersCsvDir = path.resolve(__dirname, 'fixtures', 'files', 'csv_import_users.csv');
const roomsCsvDir = path.resolve(__dirname, 'fixtures', 'files', 'csv_import_rooms.csv');
const dmMessagesCsvDir = path.resolve(__dirname, 'fixtures', 'files', 'dm_messages.csv');

const csvToJson = (): void => {
const usersCsvsToJson = (): void => {
fs.createReadStream(slackCsvDir)
.pipe(parse({ delimiter: ',', from_line: 2 }))
.on('data', (rows) => {
rowUserName.push(rows[0]);
});
fs.createReadStream(usersCsvDir)
.pipe(parse({ delimiter: ',' }))
.on('data', (rows) => {
rowUserName.push(rows[0]);
csvImportedUsernames.push(rows[0]);
});
};

const countDmMessages = (): void => {
fs.createReadStream(dmMessagesCsvDir)
.pipe(parse({ delimiter: ',' }))
.on('data', (rows) => {
dmMessages.push(rows[3]);
});
};

const roomsCsvToJson = (): void => {
fs.createReadStream(roomsCsvDir)
.pipe(parse({ delimiter: ',' }))
.on('data', (rows) => {
importedRooms.push({
name: rows[0],
ownerUsername: rows[1],
visibility: rows[2],
members: rows[3],
});
});
};

test.describe.serial('imports', () => {
test.beforeAll(() => {
csvToJson();
usersCsvsToJson();
roomsCsvToJson();
countDmMessages();
matheusbsilva137 marked this conversation as resolved.
Show resolved Hide resolved
});

test('expect import users data from slack', async ({ page }) => {
Expand All @@ -43,11 +89,70 @@ test.describe.serial('imports', () => {
});
});

test('expect to all users imported are actually listed as users', async ({ page }) => {
test('expect import users data from zipped CSV files', async ({ page }) => {
const poAdmin: Admin = new Admin(page);
await page.goto('/admin/import');

await poAdmin.btnImportNewFile.click();

await (await poAdmin.getOptionFileType('CSV')).click();

await poAdmin.inputFile.setInputFiles(zipCsvImportDir);
await poAdmin.btnImport.click();

await poAdmin.btnStartImport.click();

await expect(poAdmin.importStatusTableFirstRowCell).toBeVisible({
timeout: 30_000,
});
});

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}"`));
}
});

test('expect all imported rooms to be actually listed as rooms with correct members count', async ({ page }) => {
const poAdmin: Admin = new Admin(page);
await page.goto('/admin/rooms');

for await (const room of importedRooms) {
await poAdmin.inputSearchRooms.fill(room.name);

const expectedMembersCount = room.members.split(';').filter((username) => username !== room.ownerUsername).length + 1;
expect(page.locator(`tbody tr td:nth-child(2) >> text="${ expectedMembersCount }"`));
}
});

test('expect all imported rooms to have correct room type and owner', async ({ page }) => {
const poAdmin: Admin = new Admin(page);
await page.goto('/admin/rooms');

for await (const room of importedRooms) {
await poAdmin.inputSearchRooms.fill(room.name);
await poAdmin.getRoomRow(room.name).click();

room.visibility === 'private' ? await expect(poAdmin.privateInput).toBeChecked() : await expect(poAdmin.privateInput).not.toBeChecked();
await expect(poAdmin.roomOwnerInput).toHaveValue(room.ownerUsername);
matheusbsilva137 marked this conversation as resolved.
Show resolved Hide resolved
}
});

test('expect imported DM to be actually listed as a room with correct members and messages count', async ({ page }) => {
const poAdmin: Admin = new Admin(page);
await page.goto('/admin/rooms');

for await (const user of csvImportedUsernames) {
await poAdmin.inputSearchRooms.fill(user);
expect(page.locator(`tbody tr td:first-child >> text="${user}"`));

const expectedMembersCount = 2;
expect(page.locator(`tbody tr td:nth-child(2) >> text="${ expectedMembersCount }"`));

const expectedMessagesCount = dmMessages.length;
expect(page.locator(`tbody tr td:nth-child(3) >> text="${ expectedMessagesCount }"`));
}
});
});
8 changes: 8 additions & 0 deletions apps/meteor/tests/e2e/page-objects/admin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,18 @@ export class Admin {
return this.page.locator(`label >> text=Private`);
}

get privateInput(): Locator {
return this.page.locator('input[name="roomType"]');
}

get roomNameInput(): Locator {
return this.page.locator('input[name="roomName"]');
}

get roomOwnerInput(): Locator {
return this.page.locator('input[name="roomOwner"]');
}

get archivedLabel(): Locator {
return this.page.locator('label >> text=Archived');
}
Expand Down
Loading