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.

Sign up for GitHub

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

regression: old omnichannel rooms only being migrated after their data is loaded by the client #34090

Merged
merged 5 commits into from
Dec 2, 2024
Merged

regression: old omnichannel rooms only being migrated after their data is loaded by the client #34090

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
ac9e63c
regression: old omnichannel rooms only being migrated after their dat…
pierre-lehnen-rc Nov 29, 2024
1797ed2
remove old stuff
pierre-lehnen-rc Nov 29, 2024
a7e1a89
remove old tests
pierre-lehnen-rc Nov 29, 2024
526abcb
fixed error message expected on test
pierre-lehnen-rc Nov 29, 2024
3f580f8
improved types
pierre-lehnen-rc Nov 29, 2024
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: 1 addition & 4 deletions apps/meteor/app/api/server/v1/rooms.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@ import { composeRoomWithLastMessage } from '../helpers/composeRoomWithLastMessag
import { getPaginationItems } from '../helpers/getPaginationItems';
import { getUserFromParams } from '../helpers/getUserFromParams';
import { getUploadFormData } from '../lib/getUploadFormData';
import { maybeMigrateLivechatRoom } from '../lib/maybeMigrateLivechatRoom';
import {
findAdminRoom,
findAdminRooms,
Expand Down Expand Up @@ -442,10 +441,8 @@ API.v1.addRoute(
const { team, parentRoom } = await Team.getRoomInfo(room);
const parent = discussionParent || parentRoom;

const options = { projection: fields };

return API.v1.success({
room: (await maybeMigrateLivechatRoom(await Rooms.findOneByIdOrName(room._id, options), options)) ?? undefined,
room: await Rooms.findOneByIdOrName(room._id, { projection: fields }),
...(team && { team }),
...(parent && { parent }),
});
Expand Down
7 changes: 3 additions & 4 deletions apps/meteor/app/livechat/server/api/v1/contact.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ import { Meteor } from 'meteor/meteor';
import { API } from '../../../../api/server';
import { getPaginationItems } from '../../../../api/server/helpers/getPaginationItems';
import { createContact } from '../../lib/contacts/createContact';
import { getContactByChannel } from '../../lib/contacts/getContactByChannel';
import { getContactChannelsGrouped } from '../../lib/contacts/getContactChannelsGrouped';
import { getContactHistory } from '../../lib/contacts/getContactHistory';
import { getContacts } from '../../lib/contacts/getContacts';
Expand Down Expand Up @@ -133,13 +132,13 @@ API.v1.addRoute(
{ authRequired: true, permissionsRequired: ['view-livechat-contact'], validateParams: isGETOmnichannelContactsProps },
{
async get() {
const { contactId, visitor } = this.queryParams;
const { contactId } = this.queryParams;

if (!contactId && !visitor) {
if (!contactId) {
return API.v1.notFound();
}

const contact = await (contactId ? LivechatContacts.findOneById(contactId) : getContactByChannel(visitor));
const contact = await LivechatContacts.findOneById(contactId);

if (!contact) {
return API.v1.notFound();
Expand Down
30 changes: 0 additions & 30 deletions apps/meteor/app/livechat/server/lib/contacts/getContactByChannel.ts
View file
Open in desktop

This file was deleted.

7 changes: 6 additions & 1 deletion apps/meteor/app/livechat/server/startup.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import { callbacks } from '../../../lib/callbacks';
import { beforeLeaveRoomCallback } from '../../../lib/callbacks/beforeLeaveRoomCallback';
import { i18n } from '../../../server/lib/i18n';
import { roomCoordinator } from '../../../server/lib/rooms/roomCoordinator';
import { maybeMigrateLivechatRoom } from '../../api/server/lib/maybeMigrateLivechatRoom';
import { hasPermissionAsync } from '../../authorization/server/functions/hasPermission';
import { notifyOnUserChange } from '../../lib/server/lib/notifyListener';
import { settings } from '../../settings/server';
Expand All @@ -21,7 +22,11 @@ import './roomAccessValidator.internalService';
const logger = new Logger('LivechatStartup');

Meteor.startup(async () => {
roomCoordinator.setRoomFind('l', (_id) => LivechatRooms.findOneById(_id));
roomCoordinator.setRoomFind('l', async (_id) => {
const room = await LivechatRooms.findOneById(_id);

return (await maybeMigrateLivechatRoom(room)) ?? undefined;
});

beforeLeaveRoomCallback.add(
(user, room) => {
Expand Down
71 changes: 5 additions & 66 deletions apps/meteor/tests/end-to-end/api/livechat/contacts.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -710,7 +710,6 @@ describe('LIVECHAT - contacts', () => {
describe('[GET] omnichannel/contacts.get', () => {
let contactId: string;
let contactId2: string;
let association: ILivechatContactVisitorAssociation;

const email = faker.internet.email().toLowerCase();
const phone = faker.phone.number();
Expand Down Expand Up @@ -743,14 +742,7 @@ describe('LIVECHAT - contacts', () => {

const visitor = await createVisitor(undefined, contact.name, email, phone);

const room = await createLivechatRoom(visitor.token);
association = {
visitorId: visitor._id,
source: {
type: room.source.type,
id: room.source.id,
},
};
await createLivechatRoom(visitor.token);
});

after(async () => {
Expand All @@ -774,23 +766,6 @@ describe('LIVECHAT - contacts', () => {
expect(res.body.contact.contactManager).to.be.equal(contact.contactManager);
});

it('should be able get a contact by visitor association', async () => {
const res = await request.get(api(`omnichannel/contacts.get`)).set(credentials).query({ visitor: association });

expect(res.status).to.be.equal(200);
expect(res.body).to.have.property('success', true);
expect(res.body.contact).to.have.property('createdAt');
expect(res.body.contact._id).to.be.equal(contactId);
expect(res.body.contact.name).to.be.equal(contact.name);
expect(res.body.contact.emails).to.be.deep.equal([
{
address: contact.emails[0],
},
]);
expect(res.body.contact.phones).to.be.deep.equal([{ phoneNumber: contact.phones[0] }]);
expect(res.body.contact.contactManager).to.be.equal(contact.contactManager);
});

it('should return 404 if contact does not exist using contactId', async () => {
const res = await request.get(api(`omnichannel/contacts.get`)).set(credentials).query({ contactId: 'invalid' });

Expand All @@ -799,28 +774,6 @@ describe('LIVECHAT - contacts', () => {
expect(res.body).to.have.property('error', 'Resource not found');
});

it('should return 404 if contact does not exist using visitor association', async () => {
const res = await request
.get(api(`omnichannel/contacts.get`))
.set(credentials)
.query({ visitor: { ...association, visitorId: 'invalidId' } });

expect(res.status).to.be.equal(404);
expect(res.body).to.have.property('success', false);
expect(res.body).to.have.property('error', 'Resource not found');
});

it('should return 404 if contact does not exist using visitor source', async () => {
const res = await request
.get(api(`omnichannel/contacts.get`))
.set(credentials)
.query({ visitor: { ...association, source: { type: 'email' } } });

expect(res.status).to.be.equal(404);
expect(res.body).to.have.property('success', false);
expect(res.body).to.have.property('error', 'Resource not found');
});

it("should return an error if user doesn't have 'view-livechat-contact' permission", async () => {
await removePermissionFromAllRoles('view-livechat-contact');

Expand All @@ -835,21 +788,7 @@ describe('LIVECHAT - contacts', () => {
it('should return an error if contactId and visitor association is missing', async () => {
const res = await request.get(api(`omnichannel/contacts.get`)).set(credentials);

expectInvalidParams(res, [
"must have required property 'contactId'",
"must have required property 'visitor'",
'must match exactly one schema in oneOf [invalid-params]',
]);
});

it('should return an error if more than one field is provided', async () => {
const res = await request.get(api(`omnichannel/contacts.get`)).set(credentials).query({ contactId, visitor: association });

expectInvalidParams(res, [
'must NOT have additional properties',
'must NOT have additional properties',
'must match exactly one schema in oneOf [invalid-params]',
]);
expectInvalidParams(res, ["must have required property 'contactId' [invalid-params]"]);
});

describe('Contact Channels', () => {
Expand Down Expand Up @@ -1258,7 +1197,7 @@ describe('LIVECHAT - contacts', () => {
expect(res.status).to.be.equal(200);
expect(res.body).to.have.property('success', true);

const { body } = await request.get(api('omnichannel/contacts.get')).set(credentials).query({ visitor: association });
const { body } = await request.get(api('omnichannel/contacts.get')).set(credentials).query({ contactId: room.contactId });

expect(body.contact.channels).to.be.an('array');
expect(body.contact.channels.length).to.be.equal(1);
Expand Down Expand Up @@ -1388,7 +1327,7 @@ describe('LIVECHAT - contacts', () => {
it('should be able to unblock a contact channel', async () => {
await request.post(api('omnichannel/contacts.block')).set(credentials).send({ visitor: association });

const { body } = await request.get(api('omnichannel/contacts.get')).set(credentials).query({ visitor: association });
const { body } = await request.get(api('omnichannel/contacts.get')).set(credentials).query({ contactId: room.contactId });

expect(body.contact.channels).to.be.an('array');
expect(body.contact.channels.length).to.be.equal(1);
Expand All @@ -1399,7 +1338,7 @@ describe('LIVECHAT - contacts', () => {
expect(res.status).to.be.equal(200);
expect(res.body).to.have.property('success', true);

const { body: body2 } = await request.get(api('omnichannel/contacts.get')).set(credentials).query({ visitor: association });
const { body: body2 } = await request.get(api('omnichannel/contacts.get')).set(credentials).query({ contactId: room.contactId });

expect(body2.contact.channels).to.be.an('array');
expect(body2.contact.channels.length).to.be.equal(1);
Expand Down
30 changes: 9 additions & 21 deletions packages/rest-typings/src/v1/omnichannel.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1362,28 +1362,16 @@ export const ContactVisitorAssociationSchema = {
};

const GETOmnichannelContactsSchema = {
oneOf: [
{
type: 'object',
properties: {
contactId: {
type: 'string',
nullable: false,
isNotEmpty: true,
},
},
required: ['contactId'],
additionalProperties: false,
},
{
type: 'object',
properties: {
visitor: ContactVisitorAssociationSchema,
},
required: ['visitor'],
additionalProperties: false,
type: 'object',
properties: {
contactId: {
type: 'string',
nullable: false,
isNotEmpty: true,
},
],
},
required: ['contactId'],
additionalProperties: false,
};

export const isGETOmnichannelContactsProps = ajv.compile<GETOmnichannelContactsProps>(GETOmnichannelContactsSchema);
Expand Down
Loading