From cd021f708514c5704b83f41dddc64c453aa60778 Mon Sep 17 00:00:00 2001 From: matheusbsilva137 Date: Wed, 26 Jun 2024 21:37:04 -0300 Subject: [PATCH 01/23] feat: Allow monitors to manage departments in units they supervise --- .../imports/server/rest/departments.ts | 15 ++++-- .../app/livechat/server/lib/LivechatTyped.ts | 8 +++ .../livechat/server/methods/saveDepartment.ts | 5 +- .../livechat-enterprise/server/hooks/index.ts | 1 + .../server/hooks/manageDepartmentUnit.ts | 52 +++++++++++++++++++ .../ee/server/models/raw/LivechatUnit.ts | 29 ++++------- apps/meteor/lib/callbacks.ts | 2 + .../server/models/raw/LivechatDepartment.ts | 8 +++ .../src/models/ILivechatDepartmentModel.ts | 2 + .../src/models/ILivechatUnitModel.ts | 2 + packages/rest-typings/src/v1/omnichannel.ts | 12 ++++- 11 files changed, 111 insertions(+), 25 deletions(-) create mode 100644 apps/meteor/ee/app/livechat-enterprise/server/hooks/manageDepartmentUnit.ts diff --git a/apps/meteor/app/livechat/imports/server/rest/departments.ts b/apps/meteor/app/livechat/imports/server/rest/departments.ts index 252a83855700..e56feeac2fa3 100644 --- a/apps/meteor/app/livechat/imports/server/rest/departments.ts +++ b/apps/meteor/app/livechat/imports/server/rest/departments.ts @@ -57,10 +57,18 @@ API.v1.addRoute( check(this.bodyParams, { department: Object, agents: Match.Maybe(Array), + departmentUnit: Match.Maybe({ _id: Match.Optional(String) }), }); const agents = this.bodyParams.agents ? { upsert: this.bodyParams.agents } : {}; - const department = await LivechatTs.saveDepartment(null, this.bodyParams.department as ILivechatDepartment, agents); + const { departmentUnit } = this.bodyParams; + const department = await LivechatTs.saveDepartment( + this.userId, + null, + this.bodyParams.department as ILivechatDepartment, + agents, + departmentUnit || {}, + ); if (department) { return API.v1.success({ @@ -112,17 +120,18 @@ API.v1.addRoute( check(this.bodyParams, { department: Object, agents: Match.Maybe(Array), + departmentUnit: Match.Maybe({ _id: Match.Optional(String) }), }); const { _id } = this.urlParams; - const { department, agents } = this.bodyParams; + const { department, agents, departmentUnit } = this.bodyParams; if (!permissionToSave) { throw new Error('error-not-allowed'); } const agentParam = permissionToAddAgents && agents ? { upsert: agents } : {}; - await LivechatTs.saveDepartment(_id, department, agentParam); + await LivechatTs.saveDepartment(this.userId, _id, department, agentParam, departmentUnit || {}); return API.v1.success({ department: await LivechatDepartment.findOneById(_id), diff --git a/apps/meteor/app/livechat/server/lib/LivechatTyped.ts b/apps/meteor/app/livechat/server/lib/LivechatTyped.ts index bf5014b984f1..13c1fc3e6b42 100644 --- a/apps/meteor/app/livechat/server/lib/LivechatTyped.ts +++ b/apps/meteor/app/livechat/server/lib/LivechatTyped.ts @@ -1872,16 +1872,20 @@ class LivechatClass { * @param {string|null} _id - The department id * @param {Partial} departmentData * @param {{upsert?: { agentId: string; count?: number; order?: number; }[], remove?: { agentId: string; count?: number; order?: number; }}} [departmentAgents] - The department agents + * @param {string|undefined} departmentUnitId - The department's unit id */ async saveDepartment( + userId: string, _id: string | null, departmentData: LivechatDepartmentDTO, departmentAgents?: { upsert?: { agentId: string; count?: number; order?: number }[]; remove?: { agentId: string; count?: number; order?: number }; }, + departmentUnit?: { _id?: string }, ) { check(_id, Match.Maybe(String)); + check(departmentUnit, Match.Maybe({ _id: Match.Optional(String) })); const department = _id ? await LivechatDepartment.findOneById(_id, { projection: { _id: 1, archived: 1, enabled: 1 } }) : null; @@ -1970,6 +1974,10 @@ class LivechatClass { await callbacks.run('livechat.afterDepartmentDisabled', departmentDB); } + if (departmentUnit) { + await callbacks.run('livechat.manageDepartmentUnit', { userId, departmentId: departmentDB._id, unitId: departmentUnit._id }); + } + return departmentDB; } } diff --git a/apps/meteor/app/livechat/server/methods/saveDepartment.ts b/apps/meteor/app/livechat/server/methods/saveDepartment.ts index 971c2189f9a7..3b011a9c74a3 100644 --- a/apps/meteor/app/livechat/server/methods/saveDepartment.ts +++ b/apps/meteor/app/livechat/server/methods/saveDepartment.ts @@ -30,12 +30,13 @@ declare module '@rocket.chat/ui-contexts' { order?: number | undefined; }[] | undefined, + departmentUnit?: { _id?: string }, ) => ILivechatDepartment; } } Meteor.methods({ - async 'livechat:saveDepartment'(_id, departmentData, departmentAgents) { + async 'livechat:saveDepartment'(_id, departmentData, departmentAgents, departmentUnit) { const uid = Meteor.userId(); if (!uid || !(await hasPermissionAsync(uid, 'manage-livechat-departments'))) { throw new Meteor.Error('error-not-allowed', 'Not allowed', { @@ -43,6 +44,6 @@ Meteor.methods({ }); } - return Livechat.saveDepartment(_id, departmentData, { upsert: departmentAgents }); + return Livechat.saveDepartment(uid, _id, departmentData, { upsert: departmentAgents }, departmentUnit); }, }); diff --git a/apps/meteor/ee/app/livechat-enterprise/server/hooks/index.ts b/apps/meteor/ee/app/livechat-enterprise/server/hooks/index.ts index c5bf0a5aa392..a4b66087be2e 100644 --- a/apps/meteor/ee/app/livechat-enterprise/server/hooks/index.ts +++ b/apps/meteor/ee/app/livechat-enterprise/server/hooks/index.ts @@ -26,3 +26,4 @@ import './afterInquiryQueued'; import './sendPdfTranscriptOnClose'; import './applyRoomRestrictions'; import './afterTagRemoved'; +import './manageDepartmentUnit'; diff --git a/apps/meteor/ee/app/livechat-enterprise/server/hooks/manageDepartmentUnit.ts b/apps/meteor/ee/app/livechat-enterprise/server/hooks/manageDepartmentUnit.ts new file mode 100644 index 000000000000..1c3f7b6a782b --- /dev/null +++ b/apps/meteor/ee/app/livechat-enterprise/server/hooks/manageDepartmentUnit.ts @@ -0,0 +1,52 @@ +import type { ILivechatDepartment, IOmnichannelBusinessUnit } from '@rocket.chat/core-typings'; +import { LivechatDepartment, LivechatUnit } from '@rocket.chat/models'; + +import { hasAnyRoleAsync } from '../../../../../app/authorization/server/functions/hasRole'; +import { callbacks } from '../../../../../lib/callbacks'; +import { cbLogger } from '../lib/logger'; +import { getUnitsFromUser } from '../methods/getUnitsFromUserRoles'; + +callbacks.add( + 'livechat.manageDepartmentUnit', + async ({ userId, departmentId, unitId }) => { + const units = await getUnitsFromUser(userId); + const isLivechatManager = await hasAnyRoleAsync(userId, ['admin', 'livechat-manager']); + const department = await LivechatDepartment.findOneById>(departmentId, { + projection: { ancestors: 1, parentId: 1 }, + }); + + if (!department || (unitId && department.ancestors?.includes(unitId))) { + return; + } + + const currentDepartmentUnitId = department.parentId; + const canManageNewUnit = !unitId || isLivechatManager || (Array.isArray(units) && units.includes(unitId)); + const canManageCurrentUnit = + !currentDepartmentUnitId || isLivechatManager || (Array.isArray(units) && units.includes(currentDepartmentUnitId)); + if (!canManageNewUnit || !canManageCurrentUnit) { + return; + } + + if (currentDepartmentUnitId) { + await LivechatUnit.decrementDepartmentsCount(currentDepartmentUnitId); + } + + if (unitId) { + const unit = await LivechatUnit.findOneById>(unitId, { + projection: { ancestors: 1 }, + }); + + if (!unit) { + return; + } + + await LivechatDepartment.addDepartmentToUnit(departmentId, unitId, [unitId, ...(unit.ancestors || [])]); + await LivechatUnit.incrementDepartmentsCount(unitId); + return; + } + + await LivechatDepartment.removeDepartmentFromUnit(departmentId); + }, + callbacks.priority.HIGH, + 'livechat-manage-department-unit', +); diff --git a/apps/meteor/ee/server/models/raw/LivechatUnit.ts b/apps/meteor/ee/server/models/raw/LivechatUnit.ts index fcabf12fa4f8..c198ee04fbb0 100644 --- a/apps/meteor/ee/server/models/raw/LivechatUnit.ts +++ b/apps/meteor/ee/server/models/raw/LivechatUnit.ts @@ -11,7 +11,6 @@ const addQueryRestrictions = async (originalQuery: Filter implement // remove other departments for await (const departmentId of savedDepartments) { if (!departmentsToSave.includes(departmentId)) { - await LivechatDepartment.updateOne( - { _id: departmentId }, - { - $set: { - parentId: null, - ancestors: null, - }, - }, - ); + await LivechatDepartment.removeDepartmentFromUnit(departmentId); } } for await (const departmentId of departmentsToSave) { - await LivechatDepartment.updateOne( - { _id: departmentId }, - { - $set: { - parentId: _id, - ancestors, - }, - }, - ); + await LivechatDepartment.addDepartmentToUnit(departmentId, _id, ancestors); } await LivechatRooms.associateRoomsWithDepartmentToUnit(departmentsToSave, _id); @@ -154,6 +137,14 @@ export class LivechatUnitRaw extends BaseRaw implement return this.updateMany(query, update); } + incrementDepartmentsCount(_id: string): Promise { + return this.updateOne({ _id }, { $inc: { numDepartments: 1 } }); + } + + decrementDepartmentsCount(_id: string): Promise { + return this.updateOne({ _id }, { $inc: { numDepartments: -1 } }); + } + async removeById(_id: string): Promise { await LivechatUnitMonitors.removeByUnitId(_id); await this.removeParentAndAncestorById(_id); diff --git a/apps/meteor/lib/callbacks.ts b/apps/meteor/lib/callbacks.ts index 9b5f1e69b1d6..b620878b1981 100644 --- a/apps/meteor/lib/callbacks.ts +++ b/apps/meteor/lib/callbacks.ts @@ -208,6 +208,7 @@ type ChainedCallbackSignatures = { 'unarchiveRoom': (room: IRoom) => void; 'roomAvatarChanged': (room: IRoom) => void; 'beforeGetMentions': (mentionIds: string[], teamMentions: MessageMention[]) => Promise; + 'livechat.manageDepartmentUnit': (params: { userId: string; departmentId: string; unitId?: string }) => void; }; export type Hook = @@ -232,6 +233,7 @@ export type Hook = | 'livechat.offlineMessage' | 'livechat.onCheckRoomApiParams' | 'livechat.onLoadConfigApi' + | 'livechat.manageDepartmentUnit' | 'loginPageStateChange' | 'mapLDAPUserData' | 'onCreateUser' diff --git a/apps/meteor/server/models/raw/LivechatDepartment.ts b/apps/meteor/server/models/raw/LivechatDepartment.ts index b8263af030a8..0dea973785cf 100644 --- a/apps/meteor/server/models/raw/LivechatDepartment.ts +++ b/apps/meteor/server/models/raw/LivechatDepartment.ts @@ -222,6 +222,14 @@ export class LivechatDepartmentRaw extends BaseRaw implemen return this.updateOne({ _id }, { $set: { archived: true, enabled: false } }); } + addDepartmentToUnit(_id: string, unitId: string, ancestors: string[]): Promise { + return this.updateOne({ _id }, { $set: { parentId: unitId, ancestors } }); + } + + removeDepartmentFromUnit(_id: string): Promise { + return this.updateOne({ _id }, { $set: { parentId: null, ancestors: null } }); + } + async createOrUpdateDepartment( _id: string | null, data: { diff --git a/packages/model-typings/src/models/ILivechatDepartmentModel.ts b/packages/model-typings/src/models/ILivechatDepartmentModel.ts index 75fe0f54b2eb..bf3ea879f607 100644 --- a/packages/model-typings/src/models/ILivechatDepartmentModel.ts +++ b/packages/model-typings/src/models/ILivechatDepartmentModel.ts @@ -73,4 +73,6 @@ export interface ILivechatDepartmentModel extends IBaseModel): FindCursor; archiveDepartment(_id: string): Promise; unarchiveDepartment(_id: string): Promise; + addDepartmentToUnit(_id: string, unitId: string, ancestors: string[]): Promise; + removeDepartmentFromUnit(_id: string): Promise; } diff --git a/packages/model-typings/src/models/ILivechatUnitModel.ts b/packages/model-typings/src/models/ILivechatUnitModel.ts index 8858ee5b580e..24a482eccd0e 100644 --- a/packages/model-typings/src/models/ILivechatUnitModel.ts +++ b/packages/model-typings/src/models/ILivechatUnitModel.ts @@ -32,6 +32,8 @@ export interface ILivechatUnitModel extends IBaseModel departments: { departmentId: string }[], ): Promise>; removeParentAndAncestorById(parentId: string): Promise; + incrementDepartmentsCount(_id: string): Promise; + decrementDepartmentsCount(_id: string): Promise; removeById(_id: string): Promise; findOneByIdOrName(_idOrName: string, options: FindOptions): Promise; findByMonitorId(monitorId: string): Promise; diff --git a/packages/rest-typings/src/v1/omnichannel.ts b/packages/rest-typings/src/v1/omnichannel.ts index 3dad53cbe8d2..7a77f0ab479f 100644 --- a/packages/rest-typings/src/v1/omnichannel.ts +++ b/packages/rest-typings/src/v1/omnichannel.ts @@ -575,7 +575,8 @@ type POSTLivechatDepartmentProps = { chatClosingTags?: string[]; fallbackForwardDepartment?: string; }; - agents: { agentId: string; count?: number; order?: number }[]; + agents?: { agentId: string; count?: number; order?: number }[]; + departmentUnit?: { _id?: string }; }; const POSTLivechatDepartmentSchema = { @@ -644,6 +645,15 @@ const POSTLivechatDepartmentSchema = { }, nullable: true, }, + departmentUnit: { + type: 'object', + properties: { + _id: { + type: 'string', + }, + }, + additionalProperties: false, + }, }, required: ['department'], additionalProperties: false, From 873dd34b26cfe7310a4319e553080a1ab039a678 Mon Sep 17 00:00:00 2001 From: matheusbsilva137 Date: Wed, 26 Jun 2024 21:37:24 -0300 Subject: [PATCH 02/23] test: add end-to-end tests --- apps/meteor/tests/data/livechat/units.ts | 22 + .../tests/end-to-end/api/livechat/14-units.ts | 530 +++++++++++++++++- 2 files changed, 549 insertions(+), 3 deletions(-) diff --git a/apps/meteor/tests/data/livechat/units.ts b/apps/meteor/tests/data/livechat/units.ts index c42d7ad5b345..070293cfe9d7 100644 --- a/apps/meteor/tests/data/livechat/units.ts +++ b/apps/meteor/tests/data/livechat/units.ts @@ -46,3 +46,25 @@ export const createUnit = async (monitorId: string, username: string, department }); }); }; + +export const deleteUnit = async (unit: IOmnichannelBusinessUnit): Promise => { + return new Promise((resolve, reject) => { + request + .post(methodCall(`livechat:removeUnit`)) + .set(credentials) + .send({ + message: JSON.stringify({ + method: 'livechat:removeUnit', + params: [unit._id], + id: '101', + msg: 'method', + }), + }) + .end((err: Error, res: DummyResponse) => { + if (err) { + return reject(err); + } + resolve(JSON.parse(res.body.message).result); + }); + }); +}; diff --git a/apps/meteor/tests/end-to-end/api/livechat/14-units.ts b/apps/meteor/tests/end-to-end/api/livechat/14-units.ts index 3a03d9e93e2c..9a7993647a17 100644 --- a/apps/meteor/tests/end-to-end/api/livechat/14-units.ts +++ b/apps/meteor/tests/end-to-end/api/livechat/14-units.ts @@ -1,13 +1,16 @@ import type { ILivechatDepartment, IOmnichannelBusinessUnit } from '@rocket.chat/core-typings'; import { expect } from 'chai'; -import { before, describe, it } from 'mocha'; +import { before, after, describe, it, afterEach } from 'mocha'; import { getCredentials, api, request, credentials } from '../../../data/api-data'; +import { deleteDepartment } from '../../../data/livechat/department'; import { createDepartment } from '../../../data/livechat/rooms'; -import { createMonitor, createUnit } from '../../../data/livechat/units'; +import { createMonitor, createUnit, deleteUnit } from '../../../data/livechat/units'; import { updatePermission, updateSetting } from '../../../data/permissions.helper'; -import { createUser, deleteUser } from '../../../data/users.helper'; +import { password } from '../../../data/user'; +import { createUser, deleteUser, login } from '../../../data/users.helper'; import { IS_EE } from '../../../e2e/config/constants'; +import { updatePredictedVisitorAbandonment } from '/ee/app/livechat-enterprise/server/lib/Helper'; (IS_EE ? describe : describe.skip)('[EE] LIVECHAT - Units', function () { this.retries(0); @@ -411,4 +414,525 @@ import { IS_EE } from '../../../e2e/config/constants'; await deleteUser(user); }); }); + + describe('[POST] livechat/department', () => { + let departmentId: string; + let monitor1: Awaited> | undefined; + let monitor1Credentials: Awaited> | undefined; + let monitor2: Awaited> | undefined; + let monitor2Credentials: Awaited> | undefined; + let unit: IOmnichannelBusinessUnit; + + before(async () => { + monitor1 = await createUser(); + monitor2 = await createUser(); + await createMonitor(monitor1.username); + monitor1Credentials = await login(monitor1.username, password); + await createMonitor(monitor2.username); + monitor2Credentials = await login(monitor2.username, password); + unit = await createUnit(monitor1._id, monitor1.username, []); + }); + + after(async () => Promise.all([deleteUser(monitor1), deleteUser(monitor2), deleteUnit(unit)])); + + afterEach(() => { + if (departmentId) { + return deleteDepartment(departmentId); + } + }); + + it('should fail creating department when providing an invalid property in the department unit object', () => { + return request + .post(api('livechat/department')) + .set(credentials) + .send({ + department: { name: 'Fail-Test', enabled: true, showOnOfflineForm: true, showOnRegistration: true, email: 'bla@bla' }, + departmentUnit: { invalidProperty: true }, + }) + .expect('Content-Type', 'application/json') + .expect(400) + .expect((res) => { + expect(res.body).to.have.property('success', false); + expect(res.body).to.have.property('errorType', 'invalid-params'); + }); + }); + + it('should fail creating a department into an existing unit that a monitor does not supervise', async () => { + const departmentName = 'Test-Department-Unit2'; + await request + .post(api('livechat/department')) + .set(monitor2Credentials) + .send({ + department: { name: departmentName, enabled: true, showOnOfflineForm: true, showOnRegistration: true, email: 'bla@bla' }, + departmentUnit: { _id: unit._id }, + }) + .expect('Content-Type', 'application/json') + .expect(200) + .expect((res) => { + expect(res.body).to.have.property('success', true); + expect(res.body).to.have.property('department').that.is.an('object'); + expect(res.body.department).to.have.property('name', departmentName); + expect(res.body.department).to.have.property('type', 'd'); + expect(res.body.department).to.have.property('_id'); + departmentId = res.body.department._id; + }); + + await request + .get(api(`livechat/units/${unit._id}`)) + .set(credentials) + .expect(200) + .expect((res) => { + expect(res.body).to.have.property('_id', unit._id); + expect(res.body).to.have.property('name', unit.name); + expect(res.body).to.have.property('numMonitors', 1); + expect(res.body).to.have.property('numDepartments', 0); + expect(res.body).to.have.property('type', 'u'); + }); + + return request + .get(api(`livechat/department/${departmentId}`)) + .set(credentials) + .expect('Content-Type', 'application/json') + .expect(200) + .expect((res) => { + expect(res.body).to.have.property('success', true); + expect(res.body).to.have.property('department'); + expect(res.body.department).to.have.property('_id', departmentId); + expect(res.body.department).to.have.property('name', departmentName); + expect(res.body.department).to.not.have.property('parentId'); + expect(res.body.department).to.not.have.property('ancestors'); + }); + }); + + it('should succesfully create a department into an existing unit as a livechat manager', async () => { + const departmentName = 'Test-Department-Unit'; + await request + .post(api('livechat/department')) + .set(credentials) + .send({ + department: { name: departmentName, enabled: true, showOnOfflineForm: true, showOnRegistration: true, email: 'bla@bla' }, + departmentUnit: { _id: unit._id }, + }) + .expect('Content-Type', 'application/json') + .expect(200) + .expect((res) => { + expect(res.body).to.have.property('success', true); + expect(res.body).to.have.property('department').that.is.an('object'); + expect(res.body.department).to.have.property('name', departmentName); + expect(res.body.department).to.have.property('type', 'd'); + expect(res.body.department).to.have.property('_id'); + departmentId = res.body.department._id; + }); + + await request + .get(api(`livechat/units/${unit._id}`)) + .set(credentials) + .expect(200) + .expect((res) => { + expect(res.body).to.have.property('_id', unit._id); + expect(res.body).to.have.property('name', unit.name); + expect(res.body).to.have.property('numMonitors', 1); + expect(res.body).to.have.property('numDepartments', 1); + expect(res.body).to.have.property('type', 'u'); + }); + + return request + .get(api(`livechat/department/${departmentId}`)) + .set(credentials) + .expect('Content-Type', 'application/json') + .expect(200) + .expect((res) => { + expect(res.body).to.have.property('success', true); + expect(res.body).to.have.property('department'); + expect(res.body.department).to.have.property('_id', departmentId); + expect(res.body.department).to.have.property('name', departmentName); + expect(res.body.department).to.have.property('parentId', unit._id); + expect(res.body.department).to.have.property('ancestors').that.is.an('array').with.lengthOf(1); + expect(res.body.department.ancestors[0]).to.equal(unit._id); + }); + }); + + it('should succesfully create a department into an existing unit that a monitor supervises', async () => { + const departmentName = 'Test-Department-Unit3'; + await request + .post(api('livechat/department')) + .set(monitor1Credentials) + .send({ + department: { name: departmentName, enabled: true, showOnOfflineForm: true, showOnRegistration: true, email: 'bla@bla' }, + departmentUnit: { _id: unit._id }, + }) + .expect('Content-Type', 'application/json') + .expect(200) + .expect((res) => { + expect(res.body).to.have.property('success', true); + expect(res.body).to.have.property('department').that.is.an('object'); + expect(res.body.department).to.have.property('name', departmentName); + expect(res.body.department).to.have.property('type', 'd'); + expect(res.body.department).to.have.property('_id'); + departmentId = res.body.department._id; + }); + + await request + .get(api(`livechat/units/${unit._id}`)) + .set(credentials) + .expect(200) + .expect((res) => { + expect(res.body).to.have.property('_id', unit._id); + expect(res.body).to.have.property('name', unit.name); + expect(res.body).to.have.property('numMonitors', 1); + expect(res.body).to.have.property('numDepartments', 2); + expect(res.body).to.have.property('type', 'u'); + }); + + return request + .get(api(`livechat/department/${departmentId}`)) + .set(credentials) + .expect('Content-Type', 'application/json') + .expect(200) + .expect((res) => { + expect(res.body).to.have.property('success', true); + expect(res.body).to.have.property('department'); + expect(res.body.department).to.have.property('_id', departmentId); + expect(res.body.department).to.have.property('name', departmentName); + expect(res.body.department).to.have.property('parentId', unit._id); + expect(res.body.department).to.have.property('ancestors').that.is.an('array').with.lengthOf(1); + expect(res.body.department.ancestors[0]).to.equal(unit._id); + }); + }); + }); + + describe('[PUT] livechat/department', () => { + let monitor1: Awaited> | undefined; + let monitor1Credentials: Awaited> | undefined; + let monitor2: Awaited> | undefined; + let monitor2Credentials: Awaited> | undefined; + let unit: IOmnichannelBusinessUnit; + let department: ILivechatDepartment; + + before(async () => { + monitor1 = await createUser(); + monitor2 = await createUser(); + await createMonitor(monitor1.username); + monitor1Credentials = await login(monitor1.username, password); + await createMonitor(monitor2.username); + monitor2Credentials = await login(monitor2.username, password); + department = await createDepartment(); + unit = await createUnit(monitor1._id, monitor1.username, []); + }); + + after(async () => Promise.all([deleteUser(monitor1), deleteUser(monitor2), deleteUnit(unit), deleteDepartment(department._id)])); + + it("should fail updating a department's unit when providing an invalid property in the department unit object", () => { + const updatedName = 'updated-department-name'; + return request + .put(api(`livechat/department/${department._id}`)) + .set(credentials) + .send({ + department: { name: updatedName, enabled: true, showOnOfflineForm: true, showOnRegistration: true, email: 'bla@bla' }, + departmentUnit: { invalidProperty: true }, + }) + .expect('Content-Type', 'application/json') + .expect(400) + .expect((res) => { + expect(res.body).to.have.property('success', false); + expect(res.body).to.have.property('error', 'Match error: Unknown key in field departmentUnit.invalidProperty'); + }); + }); + + it("should fail updating a department's unit when providing an invalid _id type in the department unit object", () => { + const updatedName = 'updated-department-name'; + return request + .put(api(`livechat/department/${department._id}`)) + .set(credentials) + .send({ + department: { name: updatedName, enabled: true, showOnOfflineForm: true, showOnRegistration: true, email: 'bla@bla' }, + departmentUnit: { _id: true }, + }) + .expect('Content-Type', 'application/json') + .expect(400) + .expect((res) => { + expect(res.body).to.have.property('success', false); + expect(res.body).to.have.property('error', 'Match error: Expected string, got boolean in field departmentUnit._id'); + }); + }); + + it('should succesfully add an existing department to a unit as a livechat manager', async () => { + const updatedName = 'updated-department-name'; + await request + .put(api(`livechat/department/${department._id}`)) + .set(credentials) + .send({ + department: { name: updatedName, enabled: true, showOnOfflineForm: true, showOnRegistration: true, email: 'bla@bla' }, + departmentUnit: { _id: unit._id }, + }) + .expect('Content-Type', 'application/json') + .expect(200) + .expect((res) => { + expect(res.body).to.have.property('success', true); + expect(res.body).to.have.property('department').that.is.an('object'); + expect(res.body.department).to.have.property('name', updatedName); + expect(res.body.department).to.have.property('type', 'd'); + expect(res.body.department).to.have.property('_id', department._id); + }); + + await request + .get(api(`livechat/units/${unit._id}`)) + .set(credentials) + .expect(200) + .expect((res) => { + expect(res.body).to.have.property('_id', unit._id); + expect(res.body).to.have.property('name', unit.name); + expect(res.body).to.have.property('numMonitors', 1); + expect(res.body).to.have.property('numDepartments', 1); + expect(res.body).to.have.property('type', 'u'); + }); + + return request + .get(api(`livechat/department/${department._id}`)) + .set(credentials) + .expect('Content-Type', 'application/json') + .expect(200) + .expect((res) => { + expect(res.body).to.have.property('success', true); + expect(res.body).to.have.property('department'); + expect(res.body.department).to.have.property('_id', department._id); + expect(res.body.department).to.have.property('name', updatedName); + expect(res.body.department).to.have.property('parentId', unit._id); + expect(res.body.department).to.have.property('ancestors').that.is.an('array').with.lengthOf(1); + expect(res.body.department.ancestors[0]).to.equal(unit._id); + }); + }); + + it('should succesfully remove an existing department from a unit as a livechat manager', async () => { + const updatedName = 'updated-department-name'; + await request + .put(api(`livechat/department/${department._id}`)) + .set(credentials) + .send({ + department: { name: updatedName, enabled: true, showOnOfflineForm: true, showOnRegistration: true, email: 'bla@bla' }, + departmentUnit: {}, + }) + .expect('Content-Type', 'application/json') + .expect(200) + .expect((res) => { + expect(res.body).to.have.property('success', true); + expect(res.body).to.have.property('department').that.is.an('object'); + expect(res.body.department).to.have.property('name', updatedName); + expect(res.body.department).to.have.property('type', 'd'); + expect(res.body.department).to.have.property('_id', department._id); + }); + + await request + .get(api(`livechat/units/${unit._id}`)) + .set(credentials) + .expect(200) + .expect((res) => { + expect(res.body).to.have.property('_id', unit._id); + expect(res.body).to.have.property('name', unit.name); + expect(res.body).to.have.property('numMonitors', 1); + expect(res.body).to.have.property('numDepartments', 0); + expect(res.body).to.have.property('type', 'u'); + }); + + return request + .get(api(`livechat/department/${department._id}`)) + .set(credentials) + .expect('Content-Type', 'application/json') + .expect(200) + .expect((res) => { + expect(res.body).to.have.property('success', true); + expect(res.body).to.have.property('department'); + expect(res.body.department).to.have.property('_id', department._id); + expect(res.body.department).to.have.property('name', updatedName); + expect(res.body.department).to.have.property('parentId').that.is.null; + expect(res.body.department).to.have.property('ancestors').that.is.null; + }); + }); + + it('should fail adding a department into an existing unit that a monitor does not supervise', async () => { + const updatedName = 'updated-department-name2'; + await request + .put(api(`livechat/department/${department._id}`)) + .set(monitor2Credentials) + .send({ + department: { name: updatedName, enabled: true, showOnOfflineForm: true, showOnRegistration: true, email: 'bla@bla' }, + departmentUnit: { _id: unit._id }, + }) + .expect('Content-Type', 'application/json') + .expect(200) + .expect((res) => { + expect(res.body).to.have.property('success', true); + expect(res.body).to.have.property('department').that.is.an('object'); + expect(res.body.department).to.have.property('name', updatedName); + expect(res.body.department).to.have.property('type', 'd'); + expect(res.body.department).to.have.property('_id'); + }); + + await request + .get(api(`livechat/units/${unit._id}`)) + .set(credentials) + .expect(200) + .expect((res) => { + expect(res.body).to.have.property('_id', unit._id); + expect(res.body).to.have.property('name', unit.name); + expect(res.body).to.have.property('numMonitors', 1); + expect(res.body).to.have.property('numDepartments', 0); + expect(res.body).to.have.property('type', 'u'); + }); + + return request + .get(api(`livechat/department/${department._id}`)) + .set(credentials) + .expect('Content-Type', 'application/json') + .expect(200) + .expect((res) => { + expect(res.body).to.have.property('success', true); + expect(res.body).to.have.property('department'); + expect(res.body.department).to.have.property('_id', department._id); + expect(res.body.department).to.have.property('name', updatedName); + expect(res.body.department).to.have.property('parentId').that.is.null; + expect(res.body.department).to.have.property('ancestors').that.is.null; + }); + }); + + it('should succesfully add a department into an existing unit that a monitor supervises', async () => { + const updatedName = 'updated-department-name3'; + await request + .put(api(`livechat/department/${department._id}`)) + .set(monitor1Credentials) + .send({ + department: { name: updatedName, enabled: true, showOnOfflineForm: true, showOnRegistration: true, email: 'bla@bla' }, + departmentUnit: { _id: unit._id }, + }) + .expect('Content-Type', 'application/json') + .expect(200) + .expect((res) => { + expect(res.body).to.have.property('success', true); + expect(res.body).to.have.property('department').that.is.an('object'); + expect(res.body.department).to.have.property('name', updatedName); + expect(res.body.department).to.have.property('type', 'd'); + expect(res.body.department).to.have.property('_id'); + }); + + await request + .get(api(`livechat/units/${unit._id}`)) + .set(credentials) + .expect(200) + .expect((res) => { + expect(res.body).to.have.property('_id', unit._id); + expect(res.body).to.have.property('name', unit.name); + expect(res.body).to.have.property('numMonitors', 1); + expect(res.body).to.have.property('numDepartments', 1); + expect(res.body).to.have.property('type', 'u'); + }); + + return request + .get(api(`livechat/department/${department._id}`)) + .set(credentials) + .expect('Content-Type', 'application/json') + .expect(200) + .expect((res) => { + expect(res.body).to.have.property('success', true); + expect(res.body).to.have.property('department'); + expect(res.body.department).to.have.property('_id', department._id); + expect(res.body.department).to.have.property('name', updatedName); + expect(res.body.department).to.have.property('parentId', unit._id); + expect(res.body.department).to.have.property('ancestors').that.is.an('array').with.lengthOf(1); + expect(res.body.department.ancestors[0]).to.equal(unit._id); + }); + }); + + it('should fail removing a department from a unit that a monitor does not supervise', async () => { + const updatedName = 'updated-department-name4'; + await request + .put(api(`livechat/department/${department._id}`)) + .set(monitor2Credentials) + .send({ + department: { name: updatedName, enabled: true, showOnOfflineForm: true, showOnRegistration: true, email: 'bla@bla' }, + departmentUnit: {}, + }) + .expect('Content-Type', 'application/json') + .expect(200) + .expect((res) => { + expect(res.body).to.have.property('success', true); + expect(res.body).to.have.property('department').that.is.an('object'); + expect(res.body.department).to.have.property('name', updatedName); + expect(res.body.department).to.have.property('type', 'd'); + expect(res.body.department).to.have.property('_id'); + }); + + await request + .get(api(`livechat/units/${unit._id}`)) + .set(credentials) + .expect(200) + .expect((res) => { + expect(res.body).to.have.property('_id', unit._id); + expect(res.body).to.have.property('name', unit.name); + expect(res.body).to.have.property('numMonitors', 1); + expect(res.body).to.have.property('numDepartments', 1); + expect(res.body).to.have.property('type', 'u'); + }); + + return request + .get(api(`livechat/department/${department._id}`)) + .set(credentials) + .expect('Content-Type', 'application/json') + .expect(200) + .expect((res) => { + expect(res.body).to.have.property('success', true); + expect(res.body).to.have.property('department'); + expect(res.body.department).to.have.property('_id', department._id); + expect(res.body.department).to.have.property('name', updatedName); + expect(res.body.department).to.have.property('parentId', unit._id); + expect(res.body.department).to.have.property('ancestors').that.is.an('array').with.lengthOf(1); + expect(res.body.department.ancestors[0]).to.equal(unit._id); + }); + }); + + it('should succesfully remove a department from a unit that a monitor supervises', async () => { + const updatedName = 'updated-department-name5'; + await request + .put(api(`livechat/department/${department._id}`)) + .set(monitor1Credentials) + .send({ + department: { name: updatedName, enabled: true, showOnOfflineForm: true, showOnRegistration: true, email: 'bla@bla' }, + departmentUnit: {}, + }) + .expect('Content-Type', 'application/json') + .expect(200) + .expect((res) => { + expect(res.body).to.have.property('success', true); + expect(res.body).to.have.property('department').that.is.an('object'); + expect(res.body.department).to.have.property('name', updatedName); + expect(res.body.department).to.have.property('type', 'd'); + expect(res.body.department).to.have.property('_id'); + }); + + await request + .get(api(`livechat/units/${unit._id}`)) + .set(credentials) + .expect(200) + .expect((res) => { + expect(res.body).to.have.property('_id', unit._id); + expect(res.body).to.have.property('name', unit.name); + expect(res.body).to.have.property('numMonitors', 1); + expect(res.body).to.have.property('numDepartments', 0); + expect(res.body).to.have.property('type', 'u'); + }); + + return request + .get(api(`livechat/department/${department._id}`)) + .set(credentials) + .expect('Content-Type', 'application/json') + .expect(200) + .expect((res) => { + expect(res.body).to.have.property('success', true); + expect(res.body).to.have.property('department'); + expect(res.body.department).to.have.property('_id', department._id); + expect(res.body.department).to.have.property('name', updatedName); + expect(res.body.department).to.have.property('parentId').that.is.null; + expect(res.body.department).to.have.property('ancestors').that.is.null; + }); + }); + }); }); From 44fcb38d05ef2f1b66cbd35afd91662f6dddf84d Mon Sep 17 00:00:00 2001 From: matheusbsilva137 Date: Thu, 27 Jun 2024 10:55:06 -0300 Subject: [PATCH 03/23] fix typecheck and lint errors --- .../server/hooks/manageDepartmentUnit.ts | 1 - .../tests/end-to-end/api/livechat/14-units.ts | 17 ++++++++--------- 2 files changed, 8 insertions(+), 10 deletions(-) diff --git a/apps/meteor/ee/app/livechat-enterprise/server/hooks/manageDepartmentUnit.ts b/apps/meteor/ee/app/livechat-enterprise/server/hooks/manageDepartmentUnit.ts index 1c3f7b6a782b..c9f3e3180282 100644 --- a/apps/meteor/ee/app/livechat-enterprise/server/hooks/manageDepartmentUnit.ts +++ b/apps/meteor/ee/app/livechat-enterprise/server/hooks/manageDepartmentUnit.ts @@ -3,7 +3,6 @@ import { LivechatDepartment, LivechatUnit } from '@rocket.chat/models'; import { hasAnyRoleAsync } from '../../../../../app/authorization/server/functions/hasRole'; import { callbacks } from '../../../../../lib/callbacks'; -import { cbLogger } from '../lib/logger'; import { getUnitsFromUser } from '../methods/getUnitsFromUserRoles'; callbacks.add( diff --git a/apps/meteor/tests/end-to-end/api/livechat/14-units.ts b/apps/meteor/tests/end-to-end/api/livechat/14-units.ts index d60d17d813f1..1da565b1df9b 100644 --- a/apps/meteor/tests/end-to-end/api/livechat/14-units.ts +++ b/apps/meteor/tests/end-to-end/api/livechat/14-units.ts @@ -10,7 +10,6 @@ import { updatePermission, updateSetting } from '../../../data/permissions.helpe import { password } from '../../../data/user'; import { createUser, deleteUser, login } from '../../../data/users.helper'; import { IS_EE } from '../../../e2e/config/constants'; -import { updatePredictedVisitorAbandonment } from '/ee/app/livechat-enterprise/server/lib/Helper'; (IS_EE ? describe : describe.skip)('[EE] LIVECHAT - Units', () => { before((done) => getCredentials(done)); @@ -415,10 +414,10 @@ import { updatePredictedVisitorAbandonment } from '/ee/app/livechat-enterprise/s describe('[POST] livechat/department', () => { let departmentId: string; - let monitor1: Awaited> | undefined; - let monitor1Credentials: Awaited> | undefined; - let monitor2: Awaited> | undefined; - let monitor2Credentials: Awaited> | undefined; + let monitor1: Awaited>; + let monitor1Credentials: Awaited>; + let monitor2: Awaited>; + let monitor2Credentials: Awaited>; let unit: IOmnichannelBusinessUnit; before(async () => { @@ -600,10 +599,10 @@ import { updatePredictedVisitorAbandonment } from '/ee/app/livechat-enterprise/s }); describe('[PUT] livechat/department', () => { - let monitor1: Awaited> | undefined; - let monitor1Credentials: Awaited> | undefined; - let monitor2: Awaited> | undefined; - let monitor2Credentials: Awaited> | undefined; + let monitor1: Awaited>; + let monitor1Credentials: Awaited>; + let monitor2: Awaited>; + let monitor2Credentials: Awaited>; let unit: IOmnichannelBusinessUnit; let department: ILivechatDepartment; From f8edf183a9a4e217c4901dda7bc9ba6bc123ce1f Mon Sep 17 00:00:00 2001 From: matheusbsilva137 Date: Thu, 27 Jun 2024 11:25:30 -0300 Subject: [PATCH 04/23] fix lint --- apps/meteor/tests/data/livechat/units.ts | 38 ++++++++++++------------ 1 file changed, 19 insertions(+), 19 deletions(-) diff --git a/apps/meteor/tests/data/livechat/units.ts b/apps/meteor/tests/data/livechat/units.ts index 0e253f505fb9..26ca74b2a023 100644 --- a/apps/meteor/tests/data/livechat/units.ts +++ b/apps/meteor/tests/data/livechat/units.ts @@ -59,23 +59,23 @@ export const createUnit = async ( }; export const deleteUnit = async (unit: IOmnichannelBusinessUnit): Promise => { - return new Promise((resolve, reject) => { - request - .post(methodCall(`livechat:removeUnit`)) - .set(credentials) - .send({ - message: JSON.stringify({ - method: 'livechat:removeUnit', - params: [unit._id], - id: '101', - msg: 'method', - }), - }) - .end((err: Error, res: DummyResponse) => { - if (err) { - return reject(err); - } - resolve(JSON.parse(res.body.message).result); - }); - }); + return new Promise((resolve, reject) => { + request + .post(methodCall(`livechat:removeUnit`)) + .set(credentials) + .send({ + message: JSON.stringify({ + method: 'livechat:removeUnit', + params: [unit._id], + id: '101', + msg: 'method', + }), + }) + .end((err: Error, res: DummyResponse) => { + if (err) { + return reject(err); + } + resolve(JSON.parse(res.body.message).result); + }); + }); }; From f06e11b5e97732e20878a98edf6e72b1f9f468c0 Mon Sep 17 00:00:00 2001 From: matheusbsilva137 Date: Thu, 27 Jun 2024 12:20:40 -0300 Subject: [PATCH 05/23] Fix lint once again --- apps/meteor/tests/data/livechat/units.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/meteor/tests/data/livechat/units.ts b/apps/meteor/tests/data/livechat/units.ts index 26ca74b2a023..038a6ec8b883 100644 --- a/apps/meteor/tests/data/livechat/units.ts +++ b/apps/meteor/tests/data/livechat/units.ts @@ -60,7 +60,7 @@ export const createUnit = async ( export const deleteUnit = async (unit: IOmnichannelBusinessUnit): Promise => { return new Promise((resolve, reject) => { - request + void request .post(methodCall(`livechat:removeUnit`)) .set(credentials) .send({ From a45428792897d9f1e7b74d48631d0ca23843e4c9 Mon Sep 17 00:00:00 2001 From: matheusbsilva137 Date: Thu, 27 Jun 2024 12:21:09 -0300 Subject: [PATCH 06/23] Improve end-to-end tests --- .../tests/end-to-end/api/livechat/14-units.ts | 284 ++++-------------- 1 file changed, 63 insertions(+), 221 deletions(-) diff --git a/apps/meteor/tests/end-to-end/api/livechat/14-units.ts b/apps/meteor/tests/end-to-end/api/livechat/14-units.ts index 1da565b1df9b..9bbf0c6352bd 100644 --- a/apps/meteor/tests/end-to-end/api/livechat/14-units.ts +++ b/apps/meteor/tests/end-to-end/api/livechat/14-units.ts @@ -16,6 +16,7 @@ import { IS_EE } from '../../../e2e/config/constants'; before(async () => { await updateSetting('Livechat_enabled', true); + await updatePermission('manage-livechat-departments', ['livechat-manager', 'livechat-monitor', 'admin']); }); describe('[GET] livechat/units', () => { @@ -412,6 +413,49 @@ import { IS_EE } from '../../../e2e/config/constants'; }); }); + const testDepartmentsInUnit = (unitId: string, unitName: string, numDepartments: number) => { + return request + .get(api(`livechat/units/${unitId}`)) + .set(credentials) + .expect(200) + .expect((res) => { + expect(res.body).to.have.property('_id', unitId); + expect(res.body).to.have.property('name', unitName); + expect(res.body).to.have.property('numMonitors', 1); + expect(res.body).to.have.property('numDepartments', numDepartments); + expect(res.body).to.have.property('type', 'u'); + }); + }; + + const testDepartmentAncestors = (departmentId: string, departmentName: string, ancestor?: string | null) => { + return request + .get(api(`livechat/department/${departmentId}`)) + .set(credentials) + .expect('Content-Type', 'application/json') + .expect(200) + .expect((res) => { + expect(res.body).to.have.property('success', true); + expect(res.body).to.have.property('department'); + expect(res.body.department).to.have.property('_id', departmentId); + expect(res.body.department).to.have.property('name', departmentName); + if (ancestor) { + expect(res.body.department).to.have.property('parentId', ancestor); + expect(res.body.department).to.have.property('ancestors').that.is.an('array').with.lengthOf(1); + expect(res.body.department.ancestors[0]).to.equal(ancestor); + } + + if (ancestor === null) { + expect(res.body.department).to.have.property('parentId').that.is.null; + expect(res.body.department).to.have.property('ancestors').that.is.null; + } + + if (ancestor === undefined) { + expect(res.body.department).to.not.have.property('parentId'); + expect(res.body.department).to.not.have.property('ancestors'); + } + }); + }; + describe('[POST] livechat/department', () => { let departmentId: string; let monitor1: Awaited>; @@ -474,31 +518,9 @@ import { IS_EE } from '../../../e2e/config/constants'; departmentId = res.body.department._id; }); - await request - .get(api(`livechat/units/${unit._id}`)) - .set(credentials) - .expect(200) - .expect((res) => { - expect(res.body).to.have.property('_id', unit._id); - expect(res.body).to.have.property('name', unit.name); - expect(res.body).to.have.property('numMonitors', 1); - expect(res.body).to.have.property('numDepartments', 0); - expect(res.body).to.have.property('type', 'u'); - }); + await testDepartmentsInUnit(unit._id, unit.name, 0); - return request - .get(api(`livechat/department/${departmentId}`)) - .set(credentials) - .expect('Content-Type', 'application/json') - .expect(200) - .expect((res) => { - expect(res.body).to.have.property('success', true); - expect(res.body).to.have.property('department'); - expect(res.body.department).to.have.property('_id', departmentId); - expect(res.body.department).to.have.property('name', departmentName); - expect(res.body.department).to.not.have.property('parentId'); - expect(res.body.department).to.not.have.property('ancestors'); - }); + await testDepartmentAncestors(departmentId, departmentName); }); it('should succesfully create a department into an existing unit as a livechat manager', async () => { @@ -521,32 +543,9 @@ import { IS_EE } from '../../../e2e/config/constants'; departmentId = res.body.department._id; }); - await request - .get(api(`livechat/units/${unit._id}`)) - .set(credentials) - .expect(200) - .expect((res) => { - expect(res.body).to.have.property('_id', unit._id); - expect(res.body).to.have.property('name', unit.name); - expect(res.body).to.have.property('numMonitors', 1); - expect(res.body).to.have.property('numDepartments', 1); - expect(res.body).to.have.property('type', 'u'); - }); + await testDepartmentsInUnit(unit._id, unit.name, 1); - return request - .get(api(`livechat/department/${departmentId}`)) - .set(credentials) - .expect('Content-Type', 'application/json') - .expect(200) - .expect((res) => { - expect(res.body).to.have.property('success', true); - expect(res.body).to.have.property('department'); - expect(res.body.department).to.have.property('_id', departmentId); - expect(res.body.department).to.have.property('name', departmentName); - expect(res.body.department).to.have.property('parentId', unit._id); - expect(res.body.department).to.have.property('ancestors').that.is.an('array').with.lengthOf(1); - expect(res.body.department.ancestors[0]).to.equal(unit._id); - }); + await testDepartmentAncestors(departmentId, departmentName, unit._id); }); it('should succesfully create a department into an existing unit that a monitor supervises', async () => { @@ -569,32 +568,10 @@ import { IS_EE } from '../../../e2e/config/constants'; departmentId = res.body.department._id; }); - await request - .get(api(`livechat/units/${unit._id}`)) - .set(credentials) - .expect(200) - .expect((res) => { - expect(res.body).to.have.property('_id', unit._id); - expect(res.body).to.have.property('name', unit.name); - expect(res.body).to.have.property('numMonitors', 1); - expect(res.body).to.have.property('numDepartments', 2); - expect(res.body).to.have.property('type', 'u'); - }); + // Deleting a department currently does not decrease its unit's counter. We must adjust this check when this is fixed + await testDepartmentsInUnit(unit._id, unit.name, 2); - return request - .get(api(`livechat/department/${departmentId}`)) - .set(credentials) - .expect('Content-Type', 'application/json') - .expect(200) - .expect((res) => { - expect(res.body).to.have.property('success', true); - expect(res.body).to.have.property('department'); - expect(res.body.department).to.have.property('_id', departmentId); - expect(res.body.department).to.have.property('name', departmentName); - expect(res.body.department).to.have.property('parentId', unit._id); - expect(res.body.department).to.have.property('ancestors').that.is.an('array').with.lengthOf(1); - expect(res.body.department.ancestors[0]).to.equal(unit._id); - }); + await testDepartmentAncestors(departmentId, departmentName, unit._id); }); }); @@ -672,32 +649,9 @@ import { IS_EE } from '../../../e2e/config/constants'; expect(res.body.department).to.have.property('_id', department._id); }); - await request - .get(api(`livechat/units/${unit._id}`)) - .set(credentials) - .expect(200) - .expect((res) => { - expect(res.body).to.have.property('_id', unit._id); - expect(res.body).to.have.property('name', unit.name); - expect(res.body).to.have.property('numMonitors', 1); - expect(res.body).to.have.property('numDepartments', 1); - expect(res.body).to.have.property('type', 'u'); - }); + await testDepartmentsInUnit(unit._id, unit.name, 1); - return request - .get(api(`livechat/department/${department._id}`)) - .set(credentials) - .expect('Content-Type', 'application/json') - .expect(200) - .expect((res) => { - expect(res.body).to.have.property('success', true); - expect(res.body).to.have.property('department'); - expect(res.body.department).to.have.property('_id', department._id); - expect(res.body.department).to.have.property('name', updatedName); - expect(res.body.department).to.have.property('parentId', unit._id); - expect(res.body.department).to.have.property('ancestors').that.is.an('array').with.lengthOf(1); - expect(res.body.department.ancestors[0]).to.equal(unit._id); - }); + await testDepartmentAncestors(department._id, updatedName, unit._id); }); it('should succesfully remove an existing department from a unit as a livechat manager', async () => { @@ -719,31 +673,9 @@ import { IS_EE } from '../../../e2e/config/constants'; expect(res.body.department).to.have.property('_id', department._id); }); - await request - .get(api(`livechat/units/${unit._id}`)) - .set(credentials) - .expect(200) - .expect((res) => { - expect(res.body).to.have.property('_id', unit._id); - expect(res.body).to.have.property('name', unit.name); - expect(res.body).to.have.property('numMonitors', 1); - expect(res.body).to.have.property('numDepartments', 0); - expect(res.body).to.have.property('type', 'u'); - }); + await testDepartmentsInUnit(unit._id, unit.name, 0); - return request - .get(api(`livechat/department/${department._id}`)) - .set(credentials) - .expect('Content-Type', 'application/json') - .expect(200) - .expect((res) => { - expect(res.body).to.have.property('success', true); - expect(res.body).to.have.property('department'); - expect(res.body.department).to.have.property('_id', department._id); - expect(res.body.department).to.have.property('name', updatedName); - expect(res.body.department).to.have.property('parentId').that.is.null; - expect(res.body.department).to.have.property('ancestors').that.is.null; - }); + await testDepartmentAncestors(department._id, updatedName, null); }); it('should fail adding a department into an existing unit that a monitor does not supervise', async () => { @@ -765,31 +697,9 @@ import { IS_EE } from '../../../e2e/config/constants'; expect(res.body.department).to.have.property('_id'); }); - await request - .get(api(`livechat/units/${unit._id}`)) - .set(credentials) - .expect(200) - .expect((res) => { - expect(res.body).to.have.property('_id', unit._id); - expect(res.body).to.have.property('name', unit.name); - expect(res.body).to.have.property('numMonitors', 1); - expect(res.body).to.have.property('numDepartments', 0); - expect(res.body).to.have.property('type', 'u'); - }); + await testDepartmentsInUnit(unit._id, unit.name, 0); - return request - .get(api(`livechat/department/${department._id}`)) - .set(credentials) - .expect('Content-Type', 'application/json') - .expect(200) - .expect((res) => { - expect(res.body).to.have.property('success', true); - expect(res.body).to.have.property('department'); - expect(res.body.department).to.have.property('_id', department._id); - expect(res.body.department).to.have.property('name', updatedName); - expect(res.body.department).to.have.property('parentId').that.is.null; - expect(res.body.department).to.have.property('ancestors').that.is.null; - }); + await testDepartmentAncestors(department._id, updatedName, null); }); it('should succesfully add a department into an existing unit that a monitor supervises', async () => { @@ -811,32 +721,9 @@ import { IS_EE } from '../../../e2e/config/constants'; expect(res.body.department).to.have.property('_id'); }); - await request - .get(api(`livechat/units/${unit._id}`)) - .set(credentials) - .expect(200) - .expect((res) => { - expect(res.body).to.have.property('_id', unit._id); - expect(res.body).to.have.property('name', unit.name); - expect(res.body).to.have.property('numMonitors', 1); - expect(res.body).to.have.property('numDepartments', 1); - expect(res.body).to.have.property('type', 'u'); - }); + await testDepartmentsInUnit(unit._id, unit.name, 1); - return request - .get(api(`livechat/department/${department._id}`)) - .set(credentials) - .expect('Content-Type', 'application/json') - .expect(200) - .expect((res) => { - expect(res.body).to.have.property('success', true); - expect(res.body).to.have.property('department'); - expect(res.body.department).to.have.property('_id', department._id); - expect(res.body.department).to.have.property('name', updatedName); - expect(res.body.department).to.have.property('parentId', unit._id); - expect(res.body.department).to.have.property('ancestors').that.is.an('array').with.lengthOf(1); - expect(res.body.department.ancestors[0]).to.equal(unit._id); - }); + await testDepartmentAncestors(department._id, updatedName, unit._id); }); it('should fail removing a department from a unit that a monitor does not supervise', async () => { @@ -858,32 +745,9 @@ import { IS_EE } from '../../../e2e/config/constants'; expect(res.body.department).to.have.property('_id'); }); - await request - .get(api(`livechat/units/${unit._id}`)) - .set(credentials) - .expect(200) - .expect((res) => { - expect(res.body).to.have.property('_id', unit._id); - expect(res.body).to.have.property('name', unit.name); - expect(res.body).to.have.property('numMonitors', 1); - expect(res.body).to.have.property('numDepartments', 1); - expect(res.body).to.have.property('type', 'u'); - }); + await testDepartmentsInUnit(unit._id, unit.name, 1); - return request - .get(api(`livechat/department/${department._id}`)) - .set(credentials) - .expect('Content-Type', 'application/json') - .expect(200) - .expect((res) => { - expect(res.body).to.have.property('success', true); - expect(res.body).to.have.property('department'); - expect(res.body.department).to.have.property('_id', department._id); - expect(res.body.department).to.have.property('name', updatedName); - expect(res.body.department).to.have.property('parentId', unit._id); - expect(res.body.department).to.have.property('ancestors').that.is.an('array').with.lengthOf(1); - expect(res.body.department.ancestors[0]).to.equal(unit._id); - }); + await testDepartmentAncestors(department._id, updatedName, unit._id); }); it('should succesfully remove a department from a unit that a monitor supervises', async () => { @@ -905,31 +769,9 @@ import { IS_EE } from '../../../e2e/config/constants'; expect(res.body.department).to.have.property('_id'); }); - await request - .get(api(`livechat/units/${unit._id}`)) - .set(credentials) - .expect(200) - .expect((res) => { - expect(res.body).to.have.property('_id', unit._id); - expect(res.body).to.have.property('name', unit.name); - expect(res.body).to.have.property('numMonitors', 1); - expect(res.body).to.have.property('numDepartments', 0); - expect(res.body).to.have.property('type', 'u'); - }); + await testDepartmentsInUnit(unit._id, unit.name, 0); - return request - .get(api(`livechat/department/${department._id}`)) - .set(credentials) - .expect('Content-Type', 'application/json') - .expect(200) - .expect((res) => { - expect(res.body).to.have.property('success', true); - expect(res.body).to.have.property('department'); - expect(res.body.department).to.have.property('_id', department._id); - expect(res.body.department).to.have.property('name', updatedName); - expect(res.body.department).to.have.property('parentId').that.is.null; - expect(res.body.department).to.have.property('ancestors').that.is.null; - }); + await testDepartmentAncestors(department._id, updatedName, null); }); }); }); From f3af7c61b7c5a98cbb7382d694e3d3e1f494f991 Mon Sep 17 00:00:00 2001 From: matheusbsilva137 Date: Thu, 27 Jun 2024 15:16:40 -0300 Subject: [PATCH 07/23] tests: add end-to-end tests to livechat:saveDepartment meteor method --- .../tests/end-to-end/api/livechat/14-units.ts | 172 +++++++++++++++++- 1 file changed, 171 insertions(+), 1 deletion(-) diff --git a/apps/meteor/tests/end-to-end/api/livechat/14-units.ts b/apps/meteor/tests/end-to-end/api/livechat/14-units.ts index 9bbf0c6352bd..80b3a0b751f1 100644 --- a/apps/meteor/tests/end-to-end/api/livechat/14-units.ts +++ b/apps/meteor/tests/end-to-end/api/livechat/14-units.ts @@ -2,7 +2,7 @@ import type { ILivechatDepartment, IOmnichannelBusinessUnit } from '@rocket.chat import { expect } from 'chai'; import { before, after, describe, it, afterEach } from 'mocha'; -import { getCredentials, api, request, credentials } from '../../../data/api-data'; +import { getCredentials, api, request, credentials, methodCall } from '../../../data/api-data'; import { deleteDepartment } from '../../../data/livechat/department'; import { createDepartment } from '../../../data/livechat/rooms'; import { createMonitor, createUnit, deleteUnit } from '../../../data/livechat/units'; @@ -10,6 +10,7 @@ import { updatePermission, updateSetting } from '../../../data/permissions.helpe import { password } from '../../../data/user'; import { createUser, deleteUser, login } from '../../../data/users.helper'; import { IS_EE } from '../../../e2e/config/constants'; +import { Credentials } from '@rocket.chat/api-client'; (IS_EE ? describe : describe.skip)('[EE] LIVECHAT - Units', () => { before((done) => getCredentials(done)); @@ -774,4 +775,173 @@ import { IS_EE } from '../../../e2e/config/constants'; await testDepartmentAncestors(department._id, updatedName, null); }); }); + + describe('[POST] livechat:saveDepartment', () => { + let monitor1: Awaited>; + let monitor1Credentials: Awaited>; + let monitor2: Awaited>; + let monitor2Credentials: Awaited>; + let unit: IOmnichannelBusinessUnit; + const departmentName = 'Test-Department-Livechat-Method'; + let testDepartmentId = ''; + + const createDepartmentWithLivechatMethod = ( + userCredentials: Credentials, + departmentId: string, + departmentName: string, + departmentUnitId?: string, + ) => { + return request + .post(methodCall('livechat:saveDepartment')) + .set(userCredentials) + .send({ + message: JSON.stringify({ + method: 'livechat:saveDepartment', + params: [ + departmentId, + { name: departmentName, enabled: true, showOnOfflineForm: true, showOnRegistration: true, email: 'bla@bla' }, + [], + { _id: departmentUnitId }, + ], + id: 'id', + msg: 'method', + }), + }) + .expect('Content-Type', 'application/json') + .expect(200) + .expect((res) => { + expect(res.body).to.have.property('success', true); + expect(res.body).to.have.property('message').that.is.a('string'); + + const data = JSON.parse(res.body.message); + expect(data).to.have.property('result').that.is.an('object'); + expect(data.result).to.have.property('name', departmentName); + expect(data.result).to.have.property('type', 'd'); + expect(data.result).to.have.property('_id'); + testDepartmentId = data.result._id; + }); + }; + + before(async () => { + monitor1 = await createUser(); + monitor2 = await createUser(); + await createMonitor(monitor1.username); + monitor1Credentials = await login(monitor1.username, password); + await createMonitor(monitor2.username); + monitor2Credentials = await login(monitor2.username, password); + unit = await createUnit(monitor1._id, monitor1.username, []); + }); + + after(async () => Promise.all([deleteUser(monitor1), deleteUser(monitor2), deleteUnit(unit), deleteDepartment(testDepartmentId)])); + + it('should fail creating department when providing an invalid property in the department unit object', () => { + return request + .post(methodCall('livechat:saveDepartment')) + .set(credentials) + .send({ + message: JSON.stringify({ + method: 'livechat:saveDepartment', + params: [ + '', + { name: 'Fail-Test', enabled: true, showOnOfflineForm: true, showOnRegistration: true, email: 'bla@bla' }, + [], + { invalidProperty: true }, + ], + id: 'id', + msg: 'method', + }), + }) + .expect('Content-Type', 'application/json') + .expect(200) + .expect((res) => { + expect(res.body).to.have.property('success', true); + expect(res.body).to.have.property('message').that.is.a('string'); + const data = JSON.parse(res.body.message); + expect(data).to.have.property('error').that.is.an('object'); + expect(data.error).to.have.property('errorType', 'Match.Error'); + }); + }); + + it('should fail creating department when providing an invalid _id type in the department unit object', () => { + return request + .post(methodCall('livechat:saveDepartment')) + .set(credentials) + .send({ + message: JSON.stringify({ + method: 'livechat:saveDepartment', + params: [ + '', + { name: 'Fail-Test', enabled: true, showOnOfflineForm: true, showOnRegistration: true, email: 'bla@bla' }, + [], + { _id: true }, + ], + id: 'id', + msg: 'method', + }), + }) + .expect('Content-Type', 'application/json') + .expect(200) + .expect((res) => { + expect(res.body).to.have.property('success', true); + expect(res.body).to.have.property('message').that.is.a('string'); + const data = JSON.parse(res.body.message); + expect(data).to.have.property('error').that.is.an('object'); + expect(data.error).to.have.property('errorType', 'Match.Error'); + }); + }); + + it('should fail creating a department into an existing unit that a monitor does not supervise', async () => { + const departmentName = 'Fail-Test'; + + await createDepartmentWithLivechatMethod(monitor2Credentials, '', departmentName, unit._id); + await testDepartmentsInUnit(unit._id, unit.name, 0); + await testDepartmentAncestors(testDepartmentId, departmentName); + await deleteDepartment(testDepartmentId); + }); + + it('should succesfully create a department into an existing unit as a livechat manager', async () => { + await createDepartmentWithLivechatMethod(credentials, '', departmentName, unit._id); + await testDepartmentsInUnit(unit._id, unit.name, 1); + await testDepartmentAncestors(testDepartmentId, departmentName, unit._id); + }); + + it('should succesfully remove an existing department from a unit as a livechat manager', async () => { + await createDepartmentWithLivechatMethod(credentials, testDepartmentId, departmentName); + await testDepartmentsInUnit(unit._id, unit.name, 0); + await testDepartmentAncestors(testDepartmentId, departmentName, null); + }); + + it('should succesfully add an existing department to a unit as a livechat manager', async () => { + await createDepartmentWithLivechatMethod(credentials, testDepartmentId, departmentName, unit._id); + await testDepartmentsInUnit(unit._id, unit.name, 1); + await testDepartmentAncestors(testDepartmentId, departmentName, unit._id); + }); + + it('should succesfully remove a department from a unit that a monitor supervises', async () => { + await createDepartmentWithLivechatMethod(monitor1Credentials, testDepartmentId, departmentName); + await testDepartmentsInUnit(unit._id, unit.name, 0); + await testDepartmentAncestors(testDepartmentId, departmentName, null); + }); + + it('should succesfully add an existing department to a unit that a monitor supervises', async () => { + await createDepartmentWithLivechatMethod(monitor1Credentials, testDepartmentId, departmentName, unit._id); + await testDepartmentsInUnit(unit._id, unit.name, 1); + await testDepartmentAncestors(testDepartmentId, departmentName, unit._id); + }); + + it('should fail removing a department from a unit that a monitor does not supervise', async () => { + await createDepartmentWithLivechatMethod(monitor2Credentials, testDepartmentId, departmentName); + await testDepartmentsInUnit(unit._id, unit.name, 1); + await testDepartmentAncestors(testDepartmentId, departmentName, unit._id); + await deleteDepartment(testDepartmentId); + }); + + it('should succesfully create a department in a unit that a monitor supervises', async () => { + await createDepartmentWithLivechatMethod(monitor1Credentials, '', departmentName, unit._id); + + // Deleting a department currently does not decrease its unit's counter. We must adjust this check when this is fixed + await testDepartmentsInUnit(unit._id, unit.name, 2); + await testDepartmentAncestors(testDepartmentId, departmentName, unit._id); + }); + }); }); From b2df254e17fca9d91f390b508732daad99488a98 Mon Sep 17 00:00:00 2001 From: matheusbsilva137 Date: Thu, 27 Jun 2024 15:29:52 -0300 Subject: [PATCH 08/23] tests: improve tests legibility --- .../tests/end-to-end/api/livechat/14-units.ts | 207 +++++------------- 1 file changed, 50 insertions(+), 157 deletions(-) diff --git a/apps/meteor/tests/end-to-end/api/livechat/14-units.ts b/apps/meteor/tests/end-to-end/api/livechat/14-units.ts index 80b3a0b751f1..922408c20747 100644 --- a/apps/meteor/tests/end-to-end/api/livechat/14-units.ts +++ b/apps/meteor/tests/end-to-end/api/livechat/14-units.ts @@ -1,3 +1,4 @@ +import type { Credentials } from '@rocket.chat/api-client'; import type { ILivechatDepartment, IOmnichannelBusinessUnit } from '@rocket.chat/core-typings'; import { expect } from 'chai'; import { before, after, describe, it, afterEach } from 'mocha'; @@ -10,7 +11,6 @@ import { updatePermission, updateSetting } from '../../../data/permissions.helpe import { password } from '../../../data/user'; import { createUser, deleteUser, login } from '../../../data/users.helper'; import { IS_EE } from '../../../e2e/config/constants'; -import { Credentials } from '@rocket.chat/api-client'; (IS_EE ? describe : describe.skip)('[EE] LIVECHAT - Units', () => { before((done) => getCredentials(done)); @@ -465,6 +465,26 @@ import { Credentials } from '@rocket.chat/api-client'; let monitor2Credentials: Awaited>; let unit: IOmnichannelBusinessUnit; + const createDepartmentInUnit = (userCredentials: Credentials, departmentName: string, unitId: string) => { + return request + .post(api('livechat/department')) + .set(userCredentials) + .send({ + department: { name: departmentName, enabled: true, showOnOfflineForm: true, showOnRegistration: true, email: 'bla@bla' }, + departmentUnit: { _id: unitId }, + }) + .expect('Content-Type', 'application/json') + .expect(200) + .expect((res) => { + expect(res.body).to.have.property('success', true); + expect(res.body).to.have.property('department').that.is.an('object'); + expect(res.body.department).to.have.property('name', departmentName); + expect(res.body.department).to.have.property('type', 'd'); + expect(res.body.department).to.have.property('_id'); + departmentId = res.body.department._id; + }); + }; + before(async () => { monitor1 = await createUser(); monitor2 = await createUser(); @@ -501,77 +521,27 @@ import { Credentials } from '@rocket.chat/api-client'; it('should fail creating a department into an existing unit that a monitor does not supervise', async () => { const departmentName = 'Test-Department-Unit2'; - await request - .post(api('livechat/department')) - .set(monitor2Credentials) - .send({ - department: { name: departmentName, enabled: true, showOnOfflineForm: true, showOnRegistration: true, email: 'bla@bla' }, - departmentUnit: { _id: unit._id }, - }) - .expect('Content-Type', 'application/json') - .expect(200) - .expect((res) => { - expect(res.body).to.have.property('success', true); - expect(res.body).to.have.property('department').that.is.an('object'); - expect(res.body.department).to.have.property('name', departmentName); - expect(res.body.department).to.have.property('type', 'd'); - expect(res.body.department).to.have.property('_id'); - departmentId = res.body.department._id; - }); + await createDepartmentInUnit(monitor2Credentials, departmentName, unit._id); await testDepartmentsInUnit(unit._id, unit.name, 0); - await testDepartmentAncestors(departmentId, departmentName); }); it('should succesfully create a department into an existing unit as a livechat manager', async () => { const departmentName = 'Test-Department-Unit'; - await request - .post(api('livechat/department')) - .set(credentials) - .send({ - department: { name: departmentName, enabled: true, showOnOfflineForm: true, showOnRegistration: true, email: 'bla@bla' }, - departmentUnit: { _id: unit._id }, - }) - .expect('Content-Type', 'application/json') - .expect(200) - .expect((res) => { - expect(res.body).to.have.property('success', true); - expect(res.body).to.have.property('department').that.is.an('object'); - expect(res.body.department).to.have.property('name', departmentName); - expect(res.body.department).to.have.property('type', 'd'); - expect(res.body.department).to.have.property('_id'); - departmentId = res.body.department._id; - }); + await createDepartmentInUnit(credentials, departmentName, unit._id); await testDepartmentsInUnit(unit._id, unit.name, 1); - await testDepartmentAncestors(departmentId, departmentName, unit._id); }); it('should succesfully create a department into an existing unit that a monitor supervises', async () => { const departmentName = 'Test-Department-Unit3'; - await request - .post(api('livechat/department')) - .set(monitor1Credentials) - .send({ - department: { name: departmentName, enabled: true, showOnOfflineForm: true, showOnRegistration: true, email: 'bla@bla' }, - departmentUnit: { _id: unit._id }, - }) - .expect('Content-Type', 'application/json') - .expect(200) - .expect((res) => { - expect(res.body).to.have.property('success', true); - expect(res.body).to.have.property('department').that.is.an('object'); - expect(res.body.department).to.have.property('name', departmentName); - expect(res.body.department).to.have.property('type', 'd'); - expect(res.body.department).to.have.property('_id'); - departmentId = res.body.department._id; - }); + + await createDepartmentInUnit(monitor1Credentials, departmentName, unit._id); // Deleting a department currently does not decrease its unit's counter. We must adjust this check when this is fixed await testDepartmentsInUnit(unit._id, unit.name, 2); - await testDepartmentAncestors(departmentId, departmentName, unit._id); }); }); @@ -584,6 +554,25 @@ import { Credentials } from '@rocket.chat/api-client'; let unit: IOmnichannelBusinessUnit; let department: ILivechatDepartment; + const updateDepartment = (userCredentials: Credentials, departmentId: string, updatedName: string, unitId?: string) => { + return request + .put(api(`livechat/department/${departmentId}`)) + .set(userCredentials) + .send({ + department: { name: updatedName, enabled: true, showOnOfflineForm: true, showOnRegistration: true, email: 'bla@bla' }, + departmentUnit: { _id: unitId }, + }) + .expect('Content-Type', 'application/json') + .expect(200) + .expect((res) => { + expect(res.body).to.have.property('success', true); + expect(res.body).to.have.property('department').that.is.an('object'); + expect(res.body.department).to.have.property('name', updatedName); + expect(res.body.department).to.have.property('type', 'd'); + expect(res.body.department).to.have.property('_id', departmentId); + }); + }; + before(async () => { monitor1 = await createUser(); monitor2 = await createUser(); @@ -633,145 +622,49 @@ import { Credentials } from '@rocket.chat/api-client'; it('should succesfully add an existing department to a unit as a livechat manager', async () => { const updatedName = 'updated-department-name'; - await request - .put(api(`livechat/department/${department._id}`)) - .set(credentials) - .send({ - department: { name: updatedName, enabled: true, showOnOfflineForm: true, showOnRegistration: true, email: 'bla@bla' }, - departmentUnit: { _id: unit._id }, - }) - .expect('Content-Type', 'application/json') - .expect(200) - .expect((res) => { - expect(res.body).to.have.property('success', true); - expect(res.body).to.have.property('department').that.is.an('object'); - expect(res.body.department).to.have.property('name', updatedName); - expect(res.body.department).to.have.property('type', 'd'); - expect(res.body.department).to.have.property('_id', department._id); - }); + await updateDepartment(credentials, department._id, updatedName, unit._id); await testDepartmentsInUnit(unit._id, unit.name, 1); - await testDepartmentAncestors(department._id, updatedName, unit._id); }); it('should succesfully remove an existing department from a unit as a livechat manager', async () => { const updatedName = 'updated-department-name'; - await request - .put(api(`livechat/department/${department._id}`)) - .set(credentials) - .send({ - department: { name: updatedName, enabled: true, showOnOfflineForm: true, showOnRegistration: true, email: 'bla@bla' }, - departmentUnit: {}, - }) - .expect('Content-Type', 'application/json') - .expect(200) - .expect((res) => { - expect(res.body).to.have.property('success', true); - expect(res.body).to.have.property('department').that.is.an('object'); - expect(res.body.department).to.have.property('name', updatedName); - expect(res.body.department).to.have.property('type', 'd'); - expect(res.body.department).to.have.property('_id', department._id); - }); + await updateDepartment(credentials, department._id, updatedName); await testDepartmentsInUnit(unit._id, unit.name, 0); - await testDepartmentAncestors(department._id, updatedName, null); }); it('should fail adding a department into an existing unit that a monitor does not supervise', async () => { const updatedName = 'updated-department-name2'; - await request - .put(api(`livechat/department/${department._id}`)) - .set(monitor2Credentials) - .send({ - department: { name: updatedName, enabled: true, showOnOfflineForm: true, showOnRegistration: true, email: 'bla@bla' }, - departmentUnit: { _id: unit._id }, - }) - .expect('Content-Type', 'application/json') - .expect(200) - .expect((res) => { - expect(res.body).to.have.property('success', true); - expect(res.body).to.have.property('department').that.is.an('object'); - expect(res.body.department).to.have.property('name', updatedName); - expect(res.body.department).to.have.property('type', 'd'); - expect(res.body.department).to.have.property('_id'); - }); + await updateDepartment(monitor2Credentials, department._id, updatedName, unit._id); await testDepartmentsInUnit(unit._id, unit.name, 0); - await testDepartmentAncestors(department._id, updatedName, null); }); it('should succesfully add a department into an existing unit that a monitor supervises', async () => { const updatedName = 'updated-department-name3'; - await request - .put(api(`livechat/department/${department._id}`)) - .set(monitor1Credentials) - .send({ - department: { name: updatedName, enabled: true, showOnOfflineForm: true, showOnRegistration: true, email: 'bla@bla' }, - departmentUnit: { _id: unit._id }, - }) - .expect('Content-Type', 'application/json') - .expect(200) - .expect((res) => { - expect(res.body).to.have.property('success', true); - expect(res.body).to.have.property('department').that.is.an('object'); - expect(res.body.department).to.have.property('name', updatedName); - expect(res.body.department).to.have.property('type', 'd'); - expect(res.body.department).to.have.property('_id'); - }); + await updateDepartment(monitor1Credentials, department._id, updatedName, unit._id); await testDepartmentsInUnit(unit._id, unit.name, 1); - await testDepartmentAncestors(department._id, updatedName, unit._id); }); it('should fail removing a department from a unit that a monitor does not supervise', async () => { const updatedName = 'updated-department-name4'; - await request - .put(api(`livechat/department/${department._id}`)) - .set(monitor2Credentials) - .send({ - department: { name: updatedName, enabled: true, showOnOfflineForm: true, showOnRegistration: true, email: 'bla@bla' }, - departmentUnit: {}, - }) - .expect('Content-Type', 'application/json') - .expect(200) - .expect((res) => { - expect(res.body).to.have.property('success', true); - expect(res.body).to.have.property('department').that.is.an('object'); - expect(res.body.department).to.have.property('name', updatedName); - expect(res.body.department).to.have.property('type', 'd'); - expect(res.body.department).to.have.property('_id'); - }); + await updateDepartment(monitor2Credentials, department._id, updatedName); await testDepartmentsInUnit(unit._id, unit.name, 1); - await testDepartmentAncestors(department._id, updatedName, unit._id); }); it('should succesfully remove a department from a unit that a monitor supervises', async () => { const updatedName = 'updated-department-name5'; - await request - .put(api(`livechat/department/${department._id}`)) - .set(monitor1Credentials) - .send({ - department: { name: updatedName, enabled: true, showOnOfflineForm: true, showOnRegistration: true, email: 'bla@bla' }, - departmentUnit: {}, - }) - .expect('Content-Type', 'application/json') - .expect(200) - .expect((res) => { - expect(res.body).to.have.property('success', true); - expect(res.body).to.have.property('department').that.is.an('object'); - expect(res.body.department).to.have.property('name', updatedName); - expect(res.body.department).to.have.property('type', 'd'); - expect(res.body.department).to.have.property('_id'); - }); + await updateDepartment(monitor1Credentials, department._id, updatedName); await testDepartmentsInUnit(unit._id, unit.name, 0); - await testDepartmentAncestors(department._id, updatedName, null); }); }); From e96dfef875d52a5b438b5434ecce3316de6e18f3 Mon Sep 17 00:00:00 2001 From: Matheus Barbosa Silva <36537004+matheusbsilva137@users.noreply.github.com> Date: Thu, 27 Jun 2024 15:36:09 -0300 Subject: [PATCH 09/23] Create changeset --- .changeset/dirty-stingrays-beg.md | 7 +++++++ 1 file changed, 7 insertions(+) create mode 100644 .changeset/dirty-stingrays-beg.md diff --git a/.changeset/dirty-stingrays-beg.md b/.changeset/dirty-stingrays-beg.md new file mode 100644 index 000000000000..cf5e3a4ca839 --- /dev/null +++ b/.changeset/dirty-stingrays-beg.md @@ -0,0 +1,7 @@ +--- +"@rocket.chat/meteor": minor +"@rocket.chat/model-typings": minor +"@rocket.chat/rest-typings": minor +--- + +Added support for specifying a unit on departments' creation and update From 398aecf93a1f4a7b355eae1ad627d485e0cd5b90 Mon Sep 17 00:00:00 2001 From: matheusbsilva137 Date: Thu, 27 Jun 2024 16:05:23 -0300 Subject: [PATCH 10/23] fix jsdoc --- apps/meteor/app/livechat/server/lib/LivechatTyped.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/meteor/app/livechat/server/lib/LivechatTyped.ts b/apps/meteor/app/livechat/server/lib/LivechatTyped.ts index 2d3509c5d133..fd7adb5f4481 100644 --- a/apps/meteor/app/livechat/server/lib/LivechatTyped.ts +++ b/apps/meteor/app/livechat/server/lib/LivechatTyped.ts @@ -1879,7 +1879,7 @@ class LivechatClass { * @param {string|null} _id - The department id * @param {Partial} departmentData * @param {{upsert?: { agentId: string; count?: number; order?: number; }[], remove?: { agentId: string; count?: number; order?: number; }}} [departmentAgents] - The department agents - * @param {string|undefined} departmentUnitId - The department's unit id + * @param {{_id?: string}} [departmentUnit] - The department's unit id */ async saveDepartment( userId: string, From a46930f9ee9923f82d9a6fa38b8434d7e41e7e16 Mon Sep 17 00:00:00 2001 From: matheusbsilva137 Date: Mon, 1 Jul 2024 18:13:43 -0300 Subject: [PATCH 11/23] do not use meteor check --- apps/meteor/app/livechat/server/lib/LivechatTyped.ts | 6 +++++- apps/meteor/tests/end-to-end/api/livechat/14-units.ts | 3 ++- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/apps/meteor/app/livechat/server/lib/LivechatTyped.ts b/apps/meteor/app/livechat/server/lib/LivechatTyped.ts index fd7adb5f4481..3fc66b9ec918 100644 --- a/apps/meteor/app/livechat/server/lib/LivechatTyped.ts +++ b/apps/meteor/app/livechat/server/lib/LivechatTyped.ts @@ -1892,7 +1892,11 @@ class LivechatClass { departmentUnit?: { _id?: string }, ) { check(_id, Match.Maybe(String)); - check(departmentUnit, Match.Maybe({ _id: Match.Optional(String) })); + if (departmentUnit && !(departmentUnit._id === undefined || typeof departmentUnit._id === 'string')) { + throw new Meteor.Error('error-invalid-department-unit', 'Invalid department unit id provided', { + method: 'livechat:saveDepartment', + }); + } const department = _id ? await LivechatDepartment.findOneById(_id, { projection: { _id: 1, archived: 1, enabled: 1 } }) : null; diff --git a/apps/meteor/tests/end-to-end/api/livechat/14-units.ts b/apps/meteor/tests/end-to-end/api/livechat/14-units.ts index 922408c20747..ee2426413f9b 100644 --- a/apps/meteor/tests/end-to-end/api/livechat/14-units.ts +++ b/apps/meteor/tests/end-to-end/api/livechat/14-units.ts @@ -751,7 +751,8 @@ import { IS_EE } from '../../../e2e/config/constants'; expect(res.body).to.have.property('message').that.is.a('string'); const data = JSON.parse(res.body.message); expect(data).to.have.property('error').that.is.an('object'); - expect(data.error).to.have.property('errorType', 'Match.Error'); + expect(data.error).to.have.property('errorType', 'Meteor.Error'); + expect(data.error).to.have.property('error', 'error-invalid-department-unit'); }); }); From 1d62422d72aeacf272ad14fa6422cea60bfe8c19 Mon Sep 17 00:00:00 2001 From: matheusbsilva137 Date: Mon, 1 Jul 2024 18:36:22 -0300 Subject: [PATCH 12/23] Improve variable's names --- .../server/hooks/manageDepartmentUnit.ts | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/apps/meteor/ee/app/livechat-enterprise/server/hooks/manageDepartmentUnit.ts b/apps/meteor/ee/app/livechat-enterprise/server/hooks/manageDepartmentUnit.ts index c9f3e3180282..e94f9107de2f 100644 --- a/apps/meteor/ee/app/livechat-enterprise/server/hooks/manageDepartmentUnit.ts +++ b/apps/meteor/ee/app/livechat-enterprise/server/hooks/manageDepartmentUnit.ts @@ -8,20 +8,23 @@ import { getUnitsFromUser } from '../methods/getUnitsFromUserRoles'; callbacks.add( 'livechat.manageDepartmentUnit', async ({ userId, departmentId, unitId }) => { - const units = await getUnitsFromUser(userId); + const accessibleUnits = await getUnitsFromUser(userId); const isLivechatManager = await hasAnyRoleAsync(userId, ['admin', 'livechat-manager']); const department = await LivechatDepartment.findOneById>(departmentId, { projection: { ancestors: 1, parentId: 1 }, }); - if (!department || (unitId && department.ancestors?.includes(unitId))) { + const isDepartmentAlreadyInUnit = unitId && department?.ancestors?.includes(unitId); + if (!department || isDepartmentAlreadyInUnit) { return; } const currentDepartmentUnitId = department.parentId; - const canManageNewUnit = !unitId || isLivechatManager || (Array.isArray(units) && units.includes(unitId)); + const canManageNewUnit = !unitId || isLivechatManager || (Array.isArray(accessibleUnits) && accessibleUnits.includes(unitId)); const canManageCurrentUnit = - !currentDepartmentUnitId || isLivechatManager || (Array.isArray(units) && units.includes(currentDepartmentUnitId)); + !currentDepartmentUnitId || + isLivechatManager || + (Array.isArray(accessibleUnits) && accessibleUnits.includes(currentDepartmentUnitId)); if (!canManageNewUnit || !canManageCurrentUnit) { return; } From e2497561cee4898c64c557de8f2565f948237fe7 Mon Sep 17 00:00:00 2001 From: matheusbsilva137 Date: Tue, 2 Jul 2024 21:10:45 -0300 Subject: [PATCH 13/23] tests: Delete department in each test case instead of using afterEach --- apps/meteor/tests/end-to-end/api/livechat/14-units.ts | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/apps/meteor/tests/end-to-end/api/livechat/14-units.ts b/apps/meteor/tests/end-to-end/api/livechat/14-units.ts index ee2426413f9b..24786cf3abf9 100644 --- a/apps/meteor/tests/end-to-end/api/livechat/14-units.ts +++ b/apps/meteor/tests/end-to-end/api/livechat/14-units.ts @@ -1,7 +1,7 @@ import type { Credentials } from '@rocket.chat/api-client'; import type { ILivechatDepartment, IOmnichannelBusinessUnit } from '@rocket.chat/core-typings'; import { expect } from 'chai'; -import { before, after, describe, it, afterEach } from 'mocha'; +import { before, after, describe, it } from 'mocha'; import { getCredentials, api, request, credentials, methodCall } from '../../../data/api-data'; import { deleteDepartment } from '../../../data/livechat/department'; @@ -497,12 +497,6 @@ import { IS_EE } from '../../../e2e/config/constants'; after(async () => Promise.all([deleteUser(monitor1), deleteUser(monitor2), deleteUnit(unit)])); - afterEach(() => { - if (departmentId) { - return deleteDepartment(departmentId); - } - }); - it('should fail creating department when providing an invalid property in the department unit object', () => { return request .post(api('livechat/department')) @@ -525,6 +519,7 @@ import { IS_EE } from '../../../e2e/config/constants'; await createDepartmentInUnit(monitor2Credentials, departmentName, unit._id); await testDepartmentsInUnit(unit._id, unit.name, 0); await testDepartmentAncestors(departmentId, departmentName); + await deleteDepartment(departmentId); }); it('should succesfully create a department into an existing unit as a livechat manager', async () => { @@ -533,6 +528,7 @@ import { IS_EE } from '../../../e2e/config/constants'; await createDepartmentInUnit(credentials, departmentName, unit._id); await testDepartmentsInUnit(unit._id, unit.name, 1); await testDepartmentAncestors(departmentId, departmentName, unit._id); + await deleteDepartment(departmentId); }); it('should succesfully create a department into an existing unit that a monitor supervises', async () => { @@ -543,6 +539,7 @@ import { IS_EE } from '../../../e2e/config/constants'; // Deleting a department currently does not decrease its unit's counter. We must adjust this check when this is fixed await testDepartmentsInUnit(unit._id, unit.name, 2); await testDepartmentAncestors(departmentId, departmentName, unit._id); + await deleteDepartment(departmentId); }); }); From d75d9077f9daf5e1eb8bb0c6fe096949bc77acc1 Mon Sep 17 00:00:00 2001 From: matheusbsilva137 Date: Tue, 2 Jul 2024 21:45:38 -0300 Subject: [PATCH 14/23] tests: Use existing helper to create departments --- apps/meteor/tests/data/livechat/rooms.ts | 5 ++- .../tests/end-to-end/api/livechat/14-units.ts | 43 ++++--------------- 2 files changed, 13 insertions(+), 35 deletions(-) diff --git a/apps/meteor/tests/data/livechat/rooms.ts b/apps/meteor/tests/data/livechat/rooms.ts index e2084adda934..58945e8601fa 100644 --- a/apps/meteor/tests/data/livechat/rooms.ts +++ b/apps/meteor/tests/data/livechat/rooms.ts @@ -98,11 +98,13 @@ export const createDepartment = ( name?: string, enabled = true, opts: Record = {}, + departmentUnit?: { _id?: string }, + userCredentials: Credentials = credentials, ): Promise => { return new Promise((resolve, reject) => { void request .post(api('livechat/department')) - .set(credentials) + .set(userCredentials) .send({ department: { name: name || `Department ${Date.now()}`, @@ -113,6 +115,7 @@ export const createDepartment = ( ...opts, }, agents, + departmentUnit, }) .end((err: Error, res: DummyResponse) => { if (err) { diff --git a/apps/meteor/tests/end-to-end/api/livechat/14-units.ts b/apps/meteor/tests/end-to-end/api/livechat/14-units.ts index 24786cf3abf9..01f840880aad 100644 --- a/apps/meteor/tests/end-to-end/api/livechat/14-units.ts +++ b/apps/meteor/tests/end-to-end/api/livechat/14-units.ts @@ -458,33 +458,12 @@ import { IS_EE } from '../../../e2e/config/constants'; }; describe('[POST] livechat/department', () => { - let departmentId: string; let monitor1: Awaited>; let monitor1Credentials: Awaited>; let monitor2: Awaited>; let monitor2Credentials: Awaited>; let unit: IOmnichannelBusinessUnit; - const createDepartmentInUnit = (userCredentials: Credentials, departmentName: string, unitId: string) => { - return request - .post(api('livechat/department')) - .set(userCredentials) - .send({ - department: { name: departmentName, enabled: true, showOnOfflineForm: true, showOnRegistration: true, email: 'bla@bla' }, - departmentUnit: { _id: unitId }, - }) - .expect('Content-Type', 'application/json') - .expect(200) - .expect((res) => { - expect(res.body).to.have.property('success', true); - expect(res.body).to.have.property('department').that.is.an('object'); - expect(res.body.department).to.have.property('name', departmentName); - expect(res.body.department).to.have.property('type', 'd'); - expect(res.body.department).to.have.property('_id'); - departmentId = res.body.department._id; - }); - }; - before(async () => { monitor1 = await createUser(); monitor2 = await createUser(); @@ -514,32 +493,28 @@ import { IS_EE } from '../../../e2e/config/constants'; }); it('should fail creating a department into an existing unit that a monitor does not supervise', async () => { - const departmentName = 'Test-Department-Unit2'; + const department = await createDepartment(undefined, undefined, undefined, undefined, { _id: unit._id }, monitor2Credentials); - await createDepartmentInUnit(monitor2Credentials, departmentName, unit._id); await testDepartmentsInUnit(unit._id, unit.name, 0); - await testDepartmentAncestors(departmentId, departmentName); - await deleteDepartment(departmentId); + await testDepartmentAncestors(department._id, department.name); + await deleteDepartment(department._id); }); it('should succesfully create a department into an existing unit as a livechat manager', async () => { - const departmentName = 'Test-Department-Unit'; + const department = await createDepartment(undefined, undefined, undefined, undefined, { _id: unit._id }); - await createDepartmentInUnit(credentials, departmentName, unit._id); await testDepartmentsInUnit(unit._id, unit.name, 1); - await testDepartmentAncestors(departmentId, departmentName, unit._id); - await deleteDepartment(departmentId); + await testDepartmentAncestors(department._id, department.name, unit._id); + await deleteDepartment(department.name); }); it('should succesfully create a department into an existing unit that a monitor supervises', async () => { - const departmentName = 'Test-Department-Unit3'; - - await createDepartmentInUnit(monitor1Credentials, departmentName, unit._id); + const department = await createDepartment(undefined, undefined, undefined, undefined, { _id: unit._id }, monitor1Credentials); // Deleting a department currently does not decrease its unit's counter. We must adjust this check when this is fixed await testDepartmentsInUnit(unit._id, unit.name, 2); - await testDepartmentAncestors(departmentId, departmentName, unit._id); - await deleteDepartment(departmentId); + await testDepartmentAncestors(department._id, department.name, unit._id); + await deleteDepartment(department._id); }); }); From 9733ba65e18b163414643cd8f89ee251ea3a875a Mon Sep 17 00:00:00 2001 From: matheusbsilva137 Date: Wed, 3 Jul 2024 14:00:11 -0300 Subject: [PATCH 15/23] tests: move updateDepartment function to helpers --- apps/meteor/tests/data/livechat/rooms.ts | 42 +++++++++ .../tests/end-to-end/api/livechat/14-units.ts | 87 +++++++++++++------ 2 files changed, 103 insertions(+), 26 deletions(-) diff --git a/apps/meteor/tests/data/livechat/rooms.ts b/apps/meteor/tests/data/livechat/rooms.ts index 58945e8601fa..68444db7a418 100644 --- a/apps/meteor/tests/data/livechat/rooms.ts +++ b/apps/meteor/tests/data/livechat/rooms.ts @@ -126,6 +126,48 @@ export const createDepartment = ( }); }; +export const updateDepartment = ({ + departmentId, + userCredentials, + agents, + name, + enabled = true, + opts = {}, + departmentUnit, +}: { + departmentId: string; + userCredentials: Credentials; + agents?: { agentId: string }[]; + name?: string; + enabled?: boolean; + opts?: Record; + departmentUnit?: { _id?: string }; +}): Promise => { + return new Promise((resolve, reject) => { + void request + .put(api(`livechat/department/${departmentId}`)) + .set(userCredentials) + .send({ + department: { + name: name || `Department ${Date.now()}`, + enabled, + showOnOfflineForm: true, + showOnRegistration: true, + email: 'a@b.com', + ...opts, + }, + agents, + departmentUnit, + }) + .end((err: Error, res: DummyResponse) => { + if (err) { + return reject(err); + } + resolve(res.body.department); + }); + }); +}; + export const createAgent = (overrideUsername?: string): Promise => new Promise((resolve, reject) => { void request diff --git a/apps/meteor/tests/end-to-end/api/livechat/14-units.ts b/apps/meteor/tests/end-to-end/api/livechat/14-units.ts index 01f840880aad..9ae6b3271a66 100644 --- a/apps/meteor/tests/end-to-end/api/livechat/14-units.ts +++ b/apps/meteor/tests/end-to-end/api/livechat/14-units.ts @@ -5,7 +5,7 @@ import { before, after, describe, it } from 'mocha'; import { getCredentials, api, request, credentials, methodCall } from '../../../data/api-data'; import { deleteDepartment } from '../../../data/livechat/department'; -import { createDepartment } from '../../../data/livechat/rooms'; +import { createDepartment, updateDepartment } from '../../../data/livechat/rooms'; import { createMonitor, createUnit, deleteUnit } from '../../../data/livechat/units'; import { updatePermission, updateSetting } from '../../../data/permissions.helper'; import { password } from '../../../data/user'; @@ -526,25 +526,6 @@ import { IS_EE } from '../../../e2e/config/constants'; let unit: IOmnichannelBusinessUnit; let department: ILivechatDepartment; - const updateDepartment = (userCredentials: Credentials, departmentId: string, updatedName: string, unitId?: string) => { - return request - .put(api(`livechat/department/${departmentId}`)) - .set(userCredentials) - .send({ - department: { name: updatedName, enabled: true, showOnOfflineForm: true, showOnRegistration: true, email: 'bla@bla' }, - departmentUnit: { _id: unitId }, - }) - .expect('Content-Type', 'application/json') - .expect(200) - .expect((res) => { - expect(res.body).to.have.property('success', true); - expect(res.body).to.have.property('department').that.is.an('object'); - expect(res.body.department).to.have.property('name', updatedName); - expect(res.body.department).to.have.property('type', 'd'); - expect(res.body.department).to.have.property('_id', departmentId); - }); - }; - before(async () => { monitor1 = await createUser(); monitor2 = await createUser(); @@ -595,7 +576,16 @@ import { IS_EE } from '../../../e2e/config/constants'; it('should succesfully add an existing department to a unit as a livechat manager', async () => { const updatedName = 'updated-department-name'; - await updateDepartment(credentials, department._id, updatedName, unit._id); + const updatedDepartment = await updateDepartment({ + userCredentials: credentials, + departmentId: department._id, + name: updatedName, + departmentUnit: { _id: unit._id }, + }); + expect(updatedDepartment).to.have.property('name', updatedName); + expect(updatedDepartment).to.have.property('type', 'd'); + expect(updatedDepartment).to.have.property('_id', department._id); + await testDepartmentsInUnit(unit._id, unit.name, 1); await testDepartmentAncestors(department._id, updatedName, unit._id); }); @@ -603,7 +593,16 @@ import { IS_EE } from '../../../e2e/config/constants'; it('should succesfully remove an existing department from a unit as a livechat manager', async () => { const updatedName = 'updated-department-name'; - await updateDepartment(credentials, department._id, updatedName); + const updatedDepartment = await updateDepartment({ + userCredentials: credentials, + departmentId: department._id, + name: updatedName, + departmentUnit: {}, + }); + expect(updatedDepartment).to.have.property('name', updatedName); + expect(updatedDepartment).to.have.property('type', 'd'); + expect(updatedDepartment).to.have.property('_id', department._id); + await testDepartmentsInUnit(unit._id, unit.name, 0); await testDepartmentAncestors(department._id, updatedName, null); }); @@ -611,7 +610,16 @@ import { IS_EE } from '../../../e2e/config/constants'; it('should fail adding a department into an existing unit that a monitor does not supervise', async () => { const updatedName = 'updated-department-name2'; - await updateDepartment(monitor2Credentials, department._id, updatedName, unit._id); + const updatedDepartment = await updateDepartment({ + userCredentials: monitor2Credentials, + departmentId: department._id, + name: updatedName, + departmentUnit: { _id: unit._id }, + }); + expect(updatedDepartment).to.have.property('name', updatedName); + expect(updatedDepartment).to.have.property('type', 'd'); + expect(updatedDepartment).to.have.property('_id', department._id); + await testDepartmentsInUnit(unit._id, unit.name, 0); await testDepartmentAncestors(department._id, updatedName, null); }); @@ -619,7 +627,16 @@ import { IS_EE } from '../../../e2e/config/constants'; it('should succesfully add a department into an existing unit that a monitor supervises', async () => { const updatedName = 'updated-department-name3'; - await updateDepartment(monitor1Credentials, department._id, updatedName, unit._id); + const updatedDepartment = await updateDepartment({ + userCredentials: monitor1Credentials, + departmentId: department._id, + name: updatedName, + departmentUnit: { _id: unit._id }, + }); + expect(updatedDepartment).to.have.property('name', updatedName); + expect(updatedDepartment).to.have.property('type', 'd'); + expect(updatedDepartment).to.have.property('_id', department._id); + await testDepartmentsInUnit(unit._id, unit.name, 1); await testDepartmentAncestors(department._id, updatedName, unit._id); }); @@ -627,7 +644,16 @@ import { IS_EE } from '../../../e2e/config/constants'; it('should fail removing a department from a unit that a monitor does not supervise', async () => { const updatedName = 'updated-department-name4'; - await updateDepartment(monitor2Credentials, department._id, updatedName); + const updatedDepartment = await updateDepartment({ + userCredentials: monitor2Credentials, + departmentId: department._id, + name: updatedName, + departmentUnit: {}, + }); + expect(updatedDepartment).to.have.property('name', updatedName); + expect(updatedDepartment).to.have.property('type', 'd'); + expect(updatedDepartment).to.have.property('_id', department._id); + await testDepartmentsInUnit(unit._id, unit.name, 1); await testDepartmentAncestors(department._id, updatedName, unit._id); }); @@ -635,7 +661,16 @@ import { IS_EE } from '../../../e2e/config/constants'; it('should succesfully remove a department from a unit that a monitor supervises', async () => { const updatedName = 'updated-department-name5'; - await updateDepartment(monitor1Credentials, department._id, updatedName); + const updatedDepartment = await updateDepartment({ + userCredentials: monitor1Credentials, + departmentId: department._id, + name: updatedName, + departmentUnit: {}, + }); + expect(updatedDepartment).to.have.property('name', updatedName); + expect(updatedDepartment).to.have.property('type', 'd'); + expect(updatedDepartment).to.have.property('_id', department._id); + await testDepartmentsInUnit(unit._id, unit.name, 0); await testDepartmentAncestors(department._id, updatedName, null); }); From 35d6c421032075bc636d0d6498cbccb14320d0fc Mon Sep 17 00:00:00 2001 From: matheusbsilva137 Date: Wed, 3 Jul 2024 17:42:32 -0300 Subject: [PATCH 16/23] Fix department unit validation condition --- apps/meteor/app/livechat/server/lib/LivechatTyped.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/meteor/app/livechat/server/lib/LivechatTyped.ts b/apps/meteor/app/livechat/server/lib/LivechatTyped.ts index 3fc66b9ec918..e3891a15df58 100644 --- a/apps/meteor/app/livechat/server/lib/LivechatTyped.ts +++ b/apps/meteor/app/livechat/server/lib/LivechatTyped.ts @@ -1892,7 +1892,7 @@ class LivechatClass { departmentUnit?: { _id?: string }, ) { check(_id, Match.Maybe(String)); - if (departmentUnit && !(departmentUnit._id === undefined || typeof departmentUnit._id === 'string')) { + if (departmentUnit?._id !== undefined && typeof departmentUnit._id !== 'string') { throw new Meteor.Error('error-invalid-department-unit', 'Invalid department unit id provided', { method: 'livechat:saveDepartment', }); From b05c1d12c6f47d6ae90dab4fe30720990fa0d467 Mon Sep 17 00:00:00 2001 From: matheusbsilva137 Date: Wed, 3 Jul 2024 18:43:32 -0300 Subject: [PATCH 17/23] tests: Improve functions used in end-to-end tests --- apps/meteor/tests/data/livechat/department.ts | 29 +- apps/meteor/tests/data/livechat/units.ts | 16 +- .../tests/end-to-end/api/livechat/14-units.ts | 356 ++++++++++-------- 3 files changed, 237 insertions(+), 164 deletions(-) diff --git a/apps/meteor/tests/data/livechat/department.ts b/apps/meteor/tests/data/livechat/department.ts index 47d0f7f2b468..3ae58add602d 100644 --- a/apps/meteor/tests/data/livechat/department.ts +++ b/apps/meteor/tests/data/livechat/department.ts @@ -38,26 +38,41 @@ const updateDepartment = async (departmentId: string, departmentData: Partial
  • +export const createDepartmentWithMethod = ({ + initialAgents = [], + allowReceiveForwardOffline = false, + name, + departmentUnit, + userCredentials = credentials, + departmentId = '', +}: { + initialAgents?: { agentId: string; username: string }[]; + allowReceiveForwardOffline?: boolean; + name?: string; + departmentUnit?: { _id?: string }; + userCredentials?: Credentials; + departmentId?: string; +} = {}): Promise => new Promise((resolve, reject) => { void request .post(methodCall('livechat:saveDepartment')) - .set(credentials) + .set(userCredentials) .send({ message: JSON.stringify({ method: 'livechat:saveDepartment', params: [ - '', + departmentId, { enabled: true, email: faker.internet.email(), showOnRegistration: true, showOnOfflineForm: true, - name: `new department ${Date.now()}`, + name: name || `new department ${Date.now()}`, description: 'created from api', allowReceiveForwardOffline, }, initialAgents, + departmentUnit, ], id: 'id', msg: 'method', @@ -79,7 +94,7 @@ type OnlineAgent = { export const createDepartmentWithAnOnlineAgent = async (): Promise<{ department: ILivechatDepartment; agent: OnlineAgent }> => { const { user, credentials } = await createAnOnlineAgent(); - const department = (await createDepartmentWithMethod()) as ILivechatDepartment; + const department = await createDepartmentWithMethod(); await addOrRemoveAgentFromDepartment(department._id, { agentId: user._id, username: user.username }, true); @@ -94,7 +109,7 @@ export const createDepartmentWithAnOnlineAgent = async (): Promise<{ department: export const createDepartmentWithAgent = async (agent: OnlineAgent): Promise<{ department: ILivechatDepartment; agent: OnlineAgent }> => { const { user, credentials } = agent; - const department = (await createDepartmentWithMethod()) as ILivechatDepartment; + const department = await createDepartmentWithMethod(); await addOrRemoveAgentFromDepartment(department._id, { agentId: user._id, username: user.username }, true); @@ -137,7 +152,7 @@ export const createDepartmentWithAnOfflineAgent = async ({ }> => { const { user, credentials } = await createAnOfflineAgent(); - const department = (await createDepartmentWithMethod(undefined, allowReceiveForwardOffline)) as ILivechatDepartment; + const department = (await createDepartmentWithMethod({ allowReceiveForwardOffline })) as ILivechatDepartment; await addOrRemoveAgentFromDepartment(department._id, { agentId: user._id, username: user.username }, true); diff --git a/apps/meteor/tests/data/livechat/units.ts b/apps/meteor/tests/data/livechat/units.ts index 038a6ec8b883..8a2d0f5a833a 100644 --- a/apps/meteor/tests/data/livechat/units.ts +++ b/apps/meteor/tests/data/livechat/units.ts @@ -1,7 +1,7 @@ import { faker } from '@faker-js/faker'; import type { IOmnichannelBusinessUnit } from '@rocket.chat/core-typings'; -import { methodCall, credentials, request } from '../api-data'; +import { methodCall, credentials, request, api } from '../api-data'; import type { DummyResponse } from './utils'; export const createMonitor = async (username: string): Promise<{ _id: string; username: string }> => { @@ -79,3 +79,17 @@ export const deleteUnit = async (unit: IOmnichannelBusinessUnit): Promise => { + return new Promise((resolve, reject) => { + void request + .get(api(`livechat/units/${unitId}`)) + .set(credentials) + .end((err: Error, res: DummyResponse) => { + if (err) { + return reject(err); + } + resolve(res.body); + }); + }); +}; diff --git a/apps/meteor/tests/end-to-end/api/livechat/14-units.ts b/apps/meteor/tests/end-to-end/api/livechat/14-units.ts index 9ae6b3271a66..5123ad7f6c26 100644 --- a/apps/meteor/tests/end-to-end/api/livechat/14-units.ts +++ b/apps/meteor/tests/end-to-end/api/livechat/14-units.ts @@ -1,12 +1,11 @@ -import type { Credentials } from '@rocket.chat/api-client'; import type { ILivechatDepartment, IOmnichannelBusinessUnit } from '@rocket.chat/core-typings'; import { expect } from 'chai'; import { before, after, describe, it } from 'mocha'; import { getCredentials, api, request, credentials, methodCall } from '../../../data/api-data'; -import { deleteDepartment } from '../../../data/livechat/department'; +import { deleteDepartment, getDepartmentById, createDepartmentWithMethod } from '../../../data/livechat/department'; import { createDepartment, updateDepartment } from '../../../data/livechat/rooms'; -import { createMonitor, createUnit, deleteUnit } from '../../../data/livechat/units'; +import { createMonitor, createUnit, deleteUnit, getUnit } from '../../../data/livechat/units'; import { updatePermission, updateSetting } from '../../../data/permissions.helper'; import { password } from '../../../data/user'; import { createUser, deleteUser, login } from '../../../data/users.helper'; @@ -414,49 +413,6 @@ import { IS_EE } from '../../../e2e/config/constants'; }); }); - const testDepartmentsInUnit = (unitId: string, unitName: string, numDepartments: number) => { - return request - .get(api(`livechat/units/${unitId}`)) - .set(credentials) - .expect(200) - .expect((res) => { - expect(res.body).to.have.property('_id', unitId); - expect(res.body).to.have.property('name', unitName); - expect(res.body).to.have.property('numMonitors', 1); - expect(res.body).to.have.property('numDepartments', numDepartments); - expect(res.body).to.have.property('type', 'u'); - }); - }; - - const testDepartmentAncestors = (departmentId: string, departmentName: string, ancestor?: string | null) => { - return request - .get(api(`livechat/department/${departmentId}`)) - .set(credentials) - .expect('Content-Type', 'application/json') - .expect(200) - .expect((res) => { - expect(res.body).to.have.property('success', true); - expect(res.body).to.have.property('department'); - expect(res.body.department).to.have.property('_id', departmentId); - expect(res.body.department).to.have.property('name', departmentName); - if (ancestor) { - expect(res.body.department).to.have.property('parentId', ancestor); - expect(res.body.department).to.have.property('ancestors').that.is.an('array').with.lengthOf(1); - expect(res.body.department.ancestors[0]).to.equal(ancestor); - } - - if (ancestor === null) { - expect(res.body.department).to.have.property('parentId').that.is.null; - expect(res.body.department).to.have.property('ancestors').that.is.null; - } - - if (ancestor === undefined) { - expect(res.body.department).to.not.have.property('parentId'); - expect(res.body.department).to.not.have.property('ancestors'); - } - }); - }; - describe('[POST] livechat/department', () => { let monitor1: Awaited>; let monitor1Credentials: Awaited>; @@ -495,25 +451,48 @@ import { IS_EE } from '../../../e2e/config/constants'; it('should fail creating a department into an existing unit that a monitor does not supervise', async () => { const department = await createDepartment(undefined, undefined, undefined, undefined, { _id: unit._id }, monitor2Credentials); - await testDepartmentsInUnit(unit._id, unit.name, 0); - await testDepartmentAncestors(department._id, department.name); + const updatedUnit = await getUnit(unit._id); + expect(updatedUnit).to.have.property('name', unit.name); + expect(updatedUnit).to.have.property('numMonitors', 1); + expect(updatedUnit).to.have.property('numDepartments', 0); + + const fullDepartment = await getDepartmentById(department._id); + expect(fullDepartment).to.not.have.property('parentId'); + expect(fullDepartment).to.not.have.property('ancestors'); + await deleteDepartment(department._id); }); it('should succesfully create a department into an existing unit as a livechat manager', async () => { const department = await createDepartment(undefined, undefined, undefined, undefined, { _id: unit._id }); - await testDepartmentsInUnit(unit._id, unit.name, 1); - await testDepartmentAncestors(department._id, department.name, unit._id); - await deleteDepartment(department.name); + const updatedUnit = await getUnit(unit._id); + expect(updatedUnit).to.have.property('name', unit.name); + expect(updatedUnit).to.have.property('numMonitors', 1); + expect(updatedUnit).to.have.property('numDepartments', 1); + + const fullDepartment = await getDepartmentById(department._id); + expect(fullDepartment).to.have.property('parentId', unit._id); + expect(fullDepartment).to.have.property('ancestors').that.is.an('array').with.lengthOf(1); + expect(fullDepartment.ancestors?.[0]).to.equal(unit._id); + + await deleteDepartment(department._id); }); it('should succesfully create a department into an existing unit that a monitor supervises', async () => { const department = await createDepartment(undefined, undefined, undefined, undefined, { _id: unit._id }, monitor1Credentials); // Deleting a department currently does not decrease its unit's counter. We must adjust this check when this is fixed - await testDepartmentsInUnit(unit._id, unit.name, 2); - await testDepartmentAncestors(department._id, department.name, unit._id); + const updatedUnit = await getUnit(unit._id); + expect(updatedUnit).to.have.property('name', unit.name); + expect(updatedUnit).to.have.property('numMonitors', 1); + expect(updatedUnit).to.have.property('numDepartments', 2); + + const fullDepartment = await getDepartmentById(department._id); + expect(fullDepartment).to.have.property('parentId', unit._id); + expect(fullDepartment).to.have.property('ancestors').that.is.an('array').with.lengthOf(1); + expect(fullDepartment.ancestors?.[0]).to.equal(unit._id); + await deleteDepartment(department._id); }); }); @@ -586,8 +565,15 @@ import { IS_EE } from '../../../e2e/config/constants'; expect(updatedDepartment).to.have.property('type', 'd'); expect(updatedDepartment).to.have.property('_id', department._id); - await testDepartmentsInUnit(unit._id, unit.name, 1); - await testDepartmentAncestors(department._id, updatedName, unit._id); + const updatedUnit = await getUnit(unit._id); + expect(updatedUnit).to.have.property('name', unit.name); + expect(updatedUnit).to.have.property('numMonitors', 1); + expect(updatedUnit).to.have.property('numDepartments', 1); + + const fullDepartment = await getDepartmentById(department._id); + expect(fullDepartment).to.have.property('parentId', unit._id); + expect(fullDepartment).to.have.property('ancestors').that.is.an('array').with.lengthOf(1); + expect(fullDepartment.ancestors?.[0]).to.equal(unit._id); }); it('should succesfully remove an existing department from a unit as a livechat manager', async () => { @@ -603,8 +589,14 @@ import { IS_EE } from '../../../e2e/config/constants'; expect(updatedDepartment).to.have.property('type', 'd'); expect(updatedDepartment).to.have.property('_id', department._id); - await testDepartmentsInUnit(unit._id, unit.name, 0); - await testDepartmentAncestors(department._id, updatedName, null); + const updatedUnit = await getUnit(unit._id); + expect(updatedUnit).to.have.property('name', unit.name); + expect(updatedUnit).to.have.property('numMonitors', 1); + expect(updatedUnit).to.have.property('numDepartments', 0); + + const fullDepartment = await getDepartmentById(department._id); + expect(fullDepartment).to.have.property('parentId').that.is.null; + expect(fullDepartment).to.have.property('ancestors').that.is.null; }); it('should fail adding a department into an existing unit that a monitor does not supervise', async () => { @@ -620,8 +612,14 @@ import { IS_EE } from '../../../e2e/config/constants'; expect(updatedDepartment).to.have.property('type', 'd'); expect(updatedDepartment).to.have.property('_id', department._id); - await testDepartmentsInUnit(unit._id, unit.name, 0); - await testDepartmentAncestors(department._id, updatedName, null); + const updatedUnit = await getUnit(unit._id); + expect(updatedUnit).to.have.property('name', unit.name); + expect(updatedUnit).to.have.property('numMonitors', 1); + expect(updatedUnit).to.have.property('numDepartments', 0); + + const fullDepartment = await getDepartmentById(department._id); + expect(fullDepartment).to.have.property('parentId').that.is.null; + expect(fullDepartment).to.have.property('ancestors').that.is.null; }); it('should succesfully add a department into an existing unit that a monitor supervises', async () => { @@ -637,8 +635,16 @@ import { IS_EE } from '../../../e2e/config/constants'; expect(updatedDepartment).to.have.property('type', 'd'); expect(updatedDepartment).to.have.property('_id', department._id); - await testDepartmentsInUnit(unit._id, unit.name, 1); - await testDepartmentAncestors(department._id, updatedName, unit._id); + const updatedUnit = await getUnit(unit._id); + expect(updatedUnit).to.have.property('name', unit.name); + expect(updatedUnit).to.have.property('numMonitors', 1); + expect(updatedUnit).to.have.property('numDepartments', 1); + + const fullDepartment = await getDepartmentById(department._id); + expect(fullDepartment).to.have.property('name', updatedName); + expect(fullDepartment).to.have.property('parentId', unit._id); + expect(fullDepartment).to.have.property('ancestors').that.is.an('array').with.lengthOf(1); + expect(fullDepartment.ancestors?.[0]).to.equal(unit._id); }); it('should fail removing a department from a unit that a monitor does not supervise', async () => { @@ -654,8 +660,16 @@ import { IS_EE } from '../../../e2e/config/constants'; expect(updatedDepartment).to.have.property('type', 'd'); expect(updatedDepartment).to.have.property('_id', department._id); - await testDepartmentsInUnit(unit._id, unit.name, 1); - await testDepartmentAncestors(department._id, updatedName, unit._id); + const updatedUnit = await getUnit(unit._id); + expect(updatedUnit).to.have.property('name', unit.name); + expect(updatedUnit).to.have.property('numMonitors', 1); + expect(updatedUnit).to.have.property('numDepartments', 1); + + const fullDepartment = await getDepartmentById(department._id); + expect(fullDepartment).to.have.property('name', updatedName); + expect(fullDepartment).to.have.property('parentId', unit._id); + expect(fullDepartment).to.have.property('ancestors').that.is.an('array').with.lengthOf(1); + expect(fullDepartment.ancestors?.[0]).to.equal(unit._id); }); it('should succesfully remove a department from a unit that a monitor supervises', async () => { @@ -671,8 +685,15 @@ import { IS_EE } from '../../../e2e/config/constants'; expect(updatedDepartment).to.have.property('type', 'd'); expect(updatedDepartment).to.have.property('_id', department._id); - await testDepartmentsInUnit(unit._id, unit.name, 0); - await testDepartmentAncestors(department._id, updatedName, null); + const updatedUnit = await getUnit(unit._id); + expect(updatedUnit).to.have.property('name', unit.name); + expect(updatedUnit).to.have.property('numMonitors', 1); + expect(updatedUnit).to.have.property('numDepartments', 0); + + const fullDepartment = await getDepartmentById(department._id); + expect(fullDepartment).to.have.property('name', updatedName); + expect(fullDepartment).to.have.property('parentId').that.is.null; + expect(fullDepartment).to.have.property('ancestors').that.is.null; }); }); @@ -685,43 +706,6 @@ import { IS_EE } from '../../../e2e/config/constants'; const departmentName = 'Test-Department-Livechat-Method'; let testDepartmentId = ''; - const createDepartmentWithLivechatMethod = ( - userCredentials: Credentials, - departmentId: string, - departmentName: string, - departmentUnitId?: string, - ) => { - return request - .post(methodCall('livechat:saveDepartment')) - .set(userCredentials) - .send({ - message: JSON.stringify({ - method: 'livechat:saveDepartment', - params: [ - departmentId, - { name: departmentName, enabled: true, showOnOfflineForm: true, showOnRegistration: true, email: 'bla@bla' }, - [], - { _id: departmentUnitId }, - ], - id: 'id', - msg: 'method', - }), - }) - .expect('Content-Type', 'application/json') - .expect(200) - .expect((res) => { - expect(res.body).to.have.property('success', true); - expect(res.body).to.have.property('message').that.is.a('string'); - - const data = JSON.parse(res.body.message); - expect(data).to.have.property('result').that.is.an('object'); - expect(data.result).to.have.property('name', departmentName); - expect(data.result).to.have.property('type', 'd'); - expect(data.result).to.have.property('_id'); - testDepartmentId = data.result._id; - }); - }; - before(async () => { monitor1 = await createUser(); monitor2 = await createUser(); @@ -734,35 +718,6 @@ import { IS_EE } from '../../../e2e/config/constants'; after(async () => Promise.all([deleteUser(monitor1), deleteUser(monitor2), deleteUnit(unit), deleteDepartment(testDepartmentId)])); - it('should fail creating department when providing an invalid property in the department unit object', () => { - return request - .post(methodCall('livechat:saveDepartment')) - .set(credentials) - .send({ - message: JSON.stringify({ - method: 'livechat:saveDepartment', - params: [ - '', - { name: 'Fail-Test', enabled: true, showOnOfflineForm: true, showOnRegistration: true, email: 'bla@bla' }, - [], - { invalidProperty: true }, - ], - id: 'id', - msg: 'method', - }), - }) - .expect('Content-Type', 'application/json') - .expect(200) - .expect((res) => { - expect(res.body).to.have.property('success', true); - expect(res.body).to.have.property('message').that.is.a('string'); - const data = JSON.parse(res.body.message); - expect(data).to.have.property('error').that.is.an('object'); - expect(data.error).to.have.property('errorType', 'Meteor.Error'); - expect(data.error).to.have.property('error', 'error-invalid-department-unit'); - }); - }); - it('should fail creating department when providing an invalid _id type in the department unit object', () => { return request .post(methodCall('livechat:saveDepartment')) @@ -787,62 +742,151 @@ import { IS_EE } from '../../../e2e/config/constants'; expect(res.body).to.have.property('message').that.is.a('string'); const data = JSON.parse(res.body.message); expect(data).to.have.property('error').that.is.an('object'); - expect(data.error).to.have.property('errorType', 'Match.Error'); + expect(data.error).to.have.property('errorType', 'Meteor.Error'); + expect(data.error).to.have.property('error', 'error-invalid-department-unit'); }); }); it('should fail creating a department into an existing unit that a monitor does not supervise', async () => { const departmentName = 'Fail-Test'; - await createDepartmentWithLivechatMethod(monitor2Credentials, '', departmentName, unit._id); - await testDepartmentsInUnit(unit._id, unit.name, 0); - await testDepartmentAncestors(testDepartmentId, departmentName); + const department = await createDepartmentWithMethod({ + userCredentials: monitor2Credentials, + name: departmentName, + departmentUnit: { _id: unit._id }, + }); + testDepartmentId = department._id; + + const updatedUnit = await getUnit(unit._id); + expect(updatedUnit).to.have.property('name', unit.name); + expect(updatedUnit).to.have.property('numMonitors', 1); + expect(updatedUnit).to.have.property('numDepartments', 0); + + const fullDepartment = await getDepartmentById(testDepartmentId); + expect(fullDepartment).to.not.have.property('parentId'); + expect(fullDepartment).to.not.have.property('ancestors'); + await deleteDepartment(testDepartmentId); }); it('should succesfully create a department into an existing unit as a livechat manager', async () => { - await createDepartmentWithLivechatMethod(credentials, '', departmentName, unit._id); - await testDepartmentsInUnit(unit._id, unit.name, 1); - await testDepartmentAncestors(testDepartmentId, departmentName, unit._id); + const testDepartment = await createDepartmentWithMethod({ name: departmentName, departmentUnit: { _id: unit._id } }); + testDepartmentId = testDepartment._id; + + const updatedUnit = await getUnit(unit._id); + expect(updatedUnit).to.have.property('name', unit.name); + expect(updatedUnit).to.have.property('numMonitors', 1); + expect(updatedUnit).to.have.property('numDepartments', 1); + + const fullDepartment = await getDepartmentById(testDepartmentId); + expect(fullDepartment).to.have.property('parentId', unit._id); + expect(fullDepartment).to.have.property('ancestors').that.is.an('array').with.lengthOf(1); + expect(fullDepartment.ancestors?.[0]).to.equal(unit._id); }); it('should succesfully remove an existing department from a unit as a livechat manager', async () => { - await createDepartmentWithLivechatMethod(credentials, testDepartmentId, departmentName); - await testDepartmentsInUnit(unit._id, unit.name, 0); - await testDepartmentAncestors(testDepartmentId, departmentName, null); + await createDepartmentWithMethod({ name: departmentName, departmentUnit: {}, departmentId: testDepartmentId }); + + const updatedUnit = await getUnit(unit._id); + expect(updatedUnit).to.have.property('name', unit.name); + expect(updatedUnit).to.have.property('numMonitors', 1); + expect(updatedUnit).to.have.property('numDepartments', 0); + + const fullDepartment = await getDepartmentById(testDepartmentId); + expect(fullDepartment).to.have.property('parentId').that.is.null; + expect(fullDepartment).to.have.property('ancestors').that.is.null; }); it('should succesfully add an existing department to a unit as a livechat manager', async () => { - await createDepartmentWithLivechatMethod(credentials, testDepartmentId, departmentName, unit._id); - await testDepartmentsInUnit(unit._id, unit.name, 1); - await testDepartmentAncestors(testDepartmentId, departmentName, unit._id); + await createDepartmentWithMethod({ name: departmentName, departmentUnit: { _id: unit._id }, departmentId: testDepartmentId }); + + const updatedUnit = await getUnit(unit._id); + expect(updatedUnit).to.have.property('name', unit.name); + expect(updatedUnit).to.have.property('numMonitors', 1); + expect(updatedUnit).to.have.property('numDepartments', 1); + + const fullDepartment = await getDepartmentById(testDepartmentId); + expect(fullDepartment).to.have.property('parentId', unit._id); + expect(fullDepartment).to.have.property('ancestors').that.is.an('array').with.lengthOf(1); + expect(fullDepartment.ancestors?.[0]).to.equal(unit._id); }); it('should succesfully remove a department from a unit that a monitor supervises', async () => { - await createDepartmentWithLivechatMethod(monitor1Credentials, testDepartmentId, departmentName); - await testDepartmentsInUnit(unit._id, unit.name, 0); - await testDepartmentAncestors(testDepartmentId, departmentName, null); + await createDepartmentWithMethod({ + name: departmentName, + departmentUnit: {}, + departmentId: testDepartmentId, + userCredentials: monitor1Credentials, + }); + + const updatedUnit = await getUnit(unit._id); + expect(updatedUnit).to.have.property('name', unit.name); + expect(updatedUnit).to.have.property('numMonitors', 1); + expect(updatedUnit).to.have.property('numDepartments', 0); + + const fullDepartment = await getDepartmentById(testDepartmentId); + expect(fullDepartment).to.have.property('parentId').that.is.null; + expect(fullDepartment).to.have.property('ancestors').that.is.null; }); it('should succesfully add an existing department to a unit that a monitor supervises', async () => { - await createDepartmentWithLivechatMethod(monitor1Credentials, testDepartmentId, departmentName, unit._id); - await testDepartmentsInUnit(unit._id, unit.name, 1); - await testDepartmentAncestors(testDepartmentId, departmentName, unit._id); + await createDepartmentWithMethod({ + name: departmentName, + departmentUnit: { _id: unit._id }, + departmentId: testDepartmentId, + userCredentials: monitor1Credentials, + }); + + const updatedUnit = await getUnit(unit._id); + expect(updatedUnit).to.have.property('name', unit.name); + expect(updatedUnit).to.have.property('numMonitors', 1); + expect(updatedUnit).to.have.property('numDepartments', 1); + + const fullDepartment = await getDepartmentById(testDepartmentId); + expect(fullDepartment).to.have.property('parentId', unit._id); + expect(fullDepartment).to.have.property('ancestors').that.is.an('array').with.lengthOf(1); + expect(fullDepartment.ancestors?.[0]).to.equal(unit._id); }); it('should fail removing a department from a unit that a monitor does not supervise', async () => { - await createDepartmentWithLivechatMethod(monitor2Credentials, testDepartmentId, departmentName); - await testDepartmentsInUnit(unit._id, unit.name, 1); - await testDepartmentAncestors(testDepartmentId, departmentName, unit._id); + await createDepartmentWithMethod({ + name: departmentName, + departmentUnit: {}, + departmentId: testDepartmentId, + userCredentials: monitor2Credentials, + }); + + const updatedUnit = await getUnit(unit._id); + expect(updatedUnit).to.have.property('name', unit.name); + expect(updatedUnit).to.have.property('numMonitors', 1); + expect(updatedUnit).to.have.property('numDepartments', 1); + + const fullDepartment = await getDepartmentById(testDepartmentId); + expect(fullDepartment).to.have.property('parentId', unit._id); + expect(fullDepartment).to.have.property('ancestors').that.is.an('array').with.lengthOf(1); + expect(fullDepartment.ancestors?.[0]).to.equal(unit._id); + await deleteDepartment(testDepartmentId); }); it('should succesfully create a department in a unit that a monitor supervises', async () => { - await createDepartmentWithLivechatMethod(monitor1Credentials, '', departmentName, unit._id); + const testDepartment = await createDepartmentWithMethod({ + name: departmentName, + departmentUnit: { _id: unit._id }, + userCredentials: monitor1Credentials, + }); + testDepartmentId = testDepartment._id; // Deleting a department currently does not decrease its unit's counter. We must adjust this check when this is fixed - await testDepartmentsInUnit(unit._id, unit.name, 2); - await testDepartmentAncestors(testDepartmentId, departmentName, unit._id); + const updatedUnit = await getUnit(unit._id); + expect(updatedUnit).to.have.property('name', unit.name); + expect(updatedUnit).to.have.property('numMonitors', 1); + expect(updatedUnit).to.have.property('numDepartments', 2); + + const fullDepartment = await getDepartmentById(testDepartmentId); + expect(fullDepartment).to.have.property('parentId', unit._id); + expect(fullDepartment).to.have.property('ancestors').that.is.an('array').with.lengthOf(1); + expect(fullDepartment.ancestors?.[0]).to.equal(unit._id); }); }); }); From 9bf8c2c3474edd52bd8154711734c09c2a71023f Mon Sep 17 00:00:00 2001 From: matheusbsilva137 Date: Thu, 4 Jul 2024 14:36:35 -0300 Subject: [PATCH 18/23] tests: add unit tests --- .../server/hooks/manageDepartmentUnit.ts | 75 ++++---- .../server/hooks/manageDepartmentUnit.spec.ts | 166 ++++++++++++++++++ 2 files changed, 201 insertions(+), 40 deletions(-) create mode 100644 apps/meteor/ee/tests/unit/apps/livechat-enterprise/server/hooks/manageDepartmentUnit.spec.ts diff --git a/apps/meteor/ee/app/livechat-enterprise/server/hooks/manageDepartmentUnit.ts b/apps/meteor/ee/app/livechat-enterprise/server/hooks/manageDepartmentUnit.ts index e94f9107de2f..ac94d310db0b 100644 --- a/apps/meteor/ee/app/livechat-enterprise/server/hooks/manageDepartmentUnit.ts +++ b/apps/meteor/ee/app/livechat-enterprise/server/hooks/manageDepartmentUnit.ts @@ -5,50 +5,45 @@ import { hasAnyRoleAsync } from '../../../../../app/authorization/server/functio import { callbacks } from '../../../../../lib/callbacks'; import { getUnitsFromUser } from '../methods/getUnitsFromUserRoles'; -callbacks.add( - 'livechat.manageDepartmentUnit', - async ({ userId, departmentId, unitId }) => { - const accessibleUnits = await getUnitsFromUser(userId); - const isLivechatManager = await hasAnyRoleAsync(userId, ['admin', 'livechat-manager']); - const department = await LivechatDepartment.findOneById>(departmentId, { - projection: { ancestors: 1, parentId: 1 }, +export const manageDepartmentUnit = async ({ userId, departmentId, unitId }: { userId: string; departmentId: string; unitId: string }) => { + const accessibleUnits = await getUnitsFromUser(userId); + const isLivechatManager = await hasAnyRoleAsync(userId, ['admin', 'livechat-manager']); + const department = await LivechatDepartment.findOneById>(departmentId, { + projection: { ancestors: 1, parentId: 1 }, + }); + + const isDepartmentAlreadyInUnit = unitId && department?.ancestors?.includes(unitId); + if (!department || isDepartmentAlreadyInUnit) { + return; + } + + const currentDepartmentUnitId = department.parentId; + const canManageNewUnit = !unitId || isLivechatManager || (Array.isArray(accessibleUnits) && accessibleUnits.includes(unitId)); + const canManageCurrentUnit = + !currentDepartmentUnitId || isLivechatManager || (Array.isArray(accessibleUnits) && accessibleUnits.includes(currentDepartmentUnitId)); + if (!canManageNewUnit || !canManageCurrentUnit) { + return; + } + + if (currentDepartmentUnitId) { + await LivechatUnit.decrementDepartmentsCount(currentDepartmentUnitId); + } + + if (unitId) { + const unit = await LivechatUnit.findOneById>(unitId, { + projection: { ancestors: 1 }, }); - const isDepartmentAlreadyInUnit = unitId && department?.ancestors?.includes(unitId); - if (!department || isDepartmentAlreadyInUnit) { + if (!unit) { return; } - const currentDepartmentUnitId = department.parentId; - const canManageNewUnit = !unitId || isLivechatManager || (Array.isArray(accessibleUnits) && accessibleUnits.includes(unitId)); - const canManageCurrentUnit = - !currentDepartmentUnitId || - isLivechatManager || - (Array.isArray(accessibleUnits) && accessibleUnits.includes(currentDepartmentUnitId)); - if (!canManageNewUnit || !canManageCurrentUnit) { - return; - } - - if (currentDepartmentUnitId) { - await LivechatUnit.decrementDepartmentsCount(currentDepartmentUnitId); - } + await LivechatDepartment.addDepartmentToUnit(departmentId, unitId, [unitId, ...(unit.ancestors || [])]); + await LivechatUnit.incrementDepartmentsCount(unitId); + return; + } - if (unitId) { - const unit = await LivechatUnit.findOneById>(unitId, { - projection: { ancestors: 1 }, - }); - - if (!unit) { - return; - } - - await LivechatDepartment.addDepartmentToUnit(departmentId, unitId, [unitId, ...(unit.ancestors || [])]); - await LivechatUnit.incrementDepartmentsCount(unitId); - return; - } + await LivechatDepartment.removeDepartmentFromUnit(departmentId); +}; - await LivechatDepartment.removeDepartmentFromUnit(departmentId); - }, - callbacks.priority.HIGH, - 'livechat-manage-department-unit', -); +callbacks.add('livechat.manageDepartmentUnit', manageDepartmentUnit, callbacks.priority.HIGH, 'livechat-manage-department-unit'); diff --git a/apps/meteor/ee/tests/unit/apps/livechat-enterprise/server/hooks/manageDepartmentUnit.spec.ts b/apps/meteor/ee/tests/unit/apps/livechat-enterprise/server/hooks/manageDepartmentUnit.spec.ts new file mode 100644 index 000000000000..3bc485a94902 --- /dev/null +++ b/apps/meteor/ee/tests/unit/apps/livechat-enterprise/server/hooks/manageDepartmentUnit.spec.ts @@ -0,0 +1,166 @@ +import { expect } from 'chai'; +import { describe, it } from 'mocha'; +import proxyquire from 'proxyquire'; +import sinon from 'sinon'; + +const livechatDepartmentStub = { + findOneById: sinon.stub(), + addDepartmentToUnit: sinon.stub(), + removeDepartmentFromUnit: sinon.stub(), +}; + +const livechatUnitStub = { + findOneById: sinon.stub(), + decrementDepartmentsCount: sinon.stub(), + incrementDepartmentsCount: sinon.stub(), +}; + +const hasAnyRoleStub = sinon.stub(); +const getUnitsFromUserStub = sinon.stub(); + +const { manageDepartmentUnit } = proxyquire + .noCallThru() + .load('../../../../../../app/livechat-enterprise/server/hooks/manageDepartmentUnit.ts', { + '../methods/getUnitsFromUserRoles': { + getUnitsFromUser: getUnitsFromUserStub, + }, + '../../../../../app/authorization/server/functions/hasRole': { + hasAnyRoleAsync: hasAnyRoleStub, + }, + '@rocket.chat/models': { + LivechatDepartment: livechatDepartmentStub, + LivechatUnit: livechatUnitStub, + }, + }); + +describe('hooks/manageDepartmentUnit', () => { + beforeEach(() => { + livechatDepartmentStub.findOneById.reset(); + livechatDepartmentStub.addDepartmentToUnit.reset(); + livechatDepartmentStub.removeDepartmentFromUnit.reset(); + livechatUnitStub.findOneById.reset(); + livechatUnitStub.decrementDepartmentsCount.reset(); + livechatUnitStub.incrementDepartmentsCount.reset(); + hasAnyRoleStub.reset(); + }); + + it('should not perform any operation when an invalid department is provided', async () => { + livechatDepartmentStub.findOneById.resolves(null); + hasAnyRoleStub.resolves(true); + getUnitsFromUserStub.resolves(['unit-id']); + + await manageDepartmentUnit({ userId: 'user-id', departmentId: 'department-id', unitId: 'unit-id' }); + expect(livechatDepartmentStub.addDepartmentToUnit.notCalled).to.be.true; + expect(livechatDepartmentStub.removeDepartmentFromUnit.notCalled).to.be.true; + expect(livechatUnitStub.decrementDepartmentsCount.notCalled).to.be.true; + expect(livechatUnitStub.incrementDepartmentsCount.notCalled).to.be.true; + }); + + it('should not perform any operation if the provided department is already in unit', async () => { + livechatDepartmentStub.findOneById.resolves({ _id: 'department-id', ancestors: ['unit-id'], parentId: 'unit-id' }); + hasAnyRoleStub.resolves(true); + getUnitsFromUserStub.resolves(['unit-id']); + + await manageDepartmentUnit({ userId: 'user-id', departmentId: 'department-id', unitId: 'unit-id' }); + expect(livechatDepartmentStub.addDepartmentToUnit.notCalled).to.be.true; + expect(livechatDepartmentStub.removeDepartmentFromUnit.notCalled).to.be.true; + expect(livechatUnitStub.decrementDepartmentsCount.notCalled).to.be.true; + expect(livechatUnitStub.incrementDepartmentsCount.notCalled).to.be.true; + }); + + it("should not perform any operation if user is a monitor and can't manage new unit", async () => { + livechatDepartmentStub.findOneById.resolves({ _id: 'department-id', ancestors: ['unit-id'], parentId: 'unit-id' }); + hasAnyRoleStub.resolves(false); + getUnitsFromUserStub.resolves(['unit-id']); + + await manageDepartmentUnit({ userId: 'user-id', departmentId: 'department-id', unitId: 'new-unit-id' }); + expect(livechatDepartmentStub.addDepartmentToUnit.notCalled).to.be.true; + expect(livechatDepartmentStub.removeDepartmentFromUnit.notCalled).to.be.true; + expect(livechatUnitStub.decrementDepartmentsCount.notCalled).to.be.true; + expect(livechatUnitStub.incrementDepartmentsCount.notCalled).to.be.true; + }); + + it("should not perform any operation if user is a monitor and can't manage current unit", async () => { + livechatDepartmentStub.findOneById.resolves({ _id: 'department-id', ancestors: ['unit-id'], parentId: 'unit-id' }); + hasAnyRoleStub.resolves(false); + getUnitsFromUserStub.resolves(['new-unit-id']); + + await manageDepartmentUnit({ userId: 'user-id', departmentId: 'department-id', unitId: 'new-unit-id' }); + expect(livechatDepartmentStub.addDepartmentToUnit.notCalled).to.be.true; + expect(livechatDepartmentStub.removeDepartmentFromUnit.notCalled).to.be.true; + expect(livechatUnitStub.decrementDepartmentsCount.notCalled).to.be.true; + expect(livechatUnitStub.incrementDepartmentsCount.notCalled).to.be.true; + }); + + it('should remove department from its current unit if user is an admin/manager', async () => { + livechatDepartmentStub.findOneById.resolves({ _id: 'department-id', ancestors: ['unit-id'], parentId: 'unit-id' }); + hasAnyRoleStub.resolves(true); + getUnitsFromUserStub.resolves(undefined); + + await manageDepartmentUnit({ userId: 'user-id', departmentId: 'department-id', unitId: undefined }); + expect(livechatDepartmentStub.addDepartmentToUnit.notCalled).to.be.true; + expect(livechatUnitStub.incrementDepartmentsCount.notCalled).to.be.true; + expect(livechatDepartmentStub.removeDepartmentFromUnit.calledOnceWith('department-id')).to.be.true; + expect(livechatUnitStub.decrementDepartmentsCount.calledOnceWith('unit-id')).to.be.true; + }); + + it('should add department to a unit if user is an admin/manager', async () => { + livechatDepartmentStub.findOneById.resolves({ _id: 'department-id' }); + hasAnyRoleStub.resolves(true); + getUnitsFromUserStub.resolves(undefined); + + await manageDepartmentUnit({ userId: 'user-id', departmentId: 'department-id', unitId: 'unit-id' }); + expect(livechatDepartmentStub.addDepartmentToUnit.calledOnceWith('department-id', 'unit-id', ['unit-id'])).to.be.true; + expect(livechatUnitStub.incrementDepartmentsCount.calledOnceWith('unit-id')).to.be.true; + expect(livechatDepartmentStub.removeDepartmentFromUnit.notCalled).to.be.true; + expect(livechatUnitStub.decrementDepartmentsCount.notCalled).to.be.true; + }); + + it('should move department to a new unit if user is an admin/manager', async () => { + livechatDepartmentStub.findOneById.resolves({ _id: 'department-id', ancestors: ['unit-id'], parentId: 'unit-id' }); + hasAnyRoleStub.resolves(true); + getUnitsFromUserStub.resolves(undefined); + + await manageDepartmentUnit({ userId: 'user-id', departmentId: 'department-id', unitId: 'new-unit-id' }); + expect(livechatDepartmentStub.addDepartmentToUnit.calledOnceWith('department-id', 'new-unit-id', ['new-unit-id'])).to.be.true; + expect(livechatUnitStub.incrementDepartmentsCount.calledOnceWith('new-unit-id')).to.be.true; + expect(livechatDepartmentStub.removeDepartmentFromUnit.calledOnceWith('department-id')).to.be.true; + expect(livechatUnitStub.decrementDepartmentsCount.calledOnceWith('unit-id')).to.be.true; + }); + + it('should remove department from its current unit if user is a monitor that supervises the current unit', async () => { + livechatDepartmentStub.findOneById.resolves({ _id: 'department-id', ancestors: ['unit-id'], parentId: 'unit-id' }); + hasAnyRoleStub.resolves(false); + getUnitsFromUserStub.resolves(['unit-id']); + + await manageDepartmentUnit({ userId: 'user-id', departmentId: 'department-id', unitId: undefined }); + expect(livechatDepartmentStub.addDepartmentToUnit.notCalled).to.be.true; + expect(livechatUnitStub.incrementDepartmentsCount.notCalled).to.be.true; + expect(livechatDepartmentStub.removeDepartmentFromUnit.calledOnceWith('department-id')).to.be.true; + expect(livechatUnitStub.decrementDepartmentsCount.calledOnceWith('unit-id')).to.be.true; + }); + + it('should add department to a unit if user is a monitor that supervises the new unit', async () => { + livechatDepartmentStub.findOneById.resolves({ _id: 'department-id' }); + hasAnyRoleStub.resolves(false); + getUnitsFromUserStub.resolves(['unit-id']); + + await manageDepartmentUnit({ userId: 'user-id', departmentId: 'department-id', unitId: 'unit-id' }); + expect(livechatDepartmentStub.addDepartmentToUnit.calledOnceWith('department-id', 'unit-id', ['unit-id'])).to.be.true; + expect(livechatUnitStub.incrementDepartmentsCount.calledOnceWith('unit-id')).to.be.true; + expect(livechatDepartmentStub.removeDepartmentFromUnit.notCalled).to.be.true; + expect(livechatUnitStub.decrementDepartmentsCount.notCalled).to.be.true; + }); + + it('should move department to a new unit if user is a monitor that supervises the current and new units', async () => { + livechatDepartmentStub.findOneById.resolves({ _id: 'department-id', ancestors: ['unit-id'], parentId: 'unit-id' }); + hasAnyRoleStub.resolves(false); + getUnitsFromUserStub.resolves(['unit-id', 'new-unit']); + + await manageDepartmentUnit({ userId: 'user-id', departmentId: 'department-id', unitId: 'new-unit-id' }); + expect(livechatDepartmentStub.addDepartmentToUnit.calledOnceWith('department-id', 'new-unit-id', ['new-unit-id'])).to.be.true; + expect(livechatUnitStub.incrementDepartmentsCount.calledOnceWith('new-unit-id')).to.be.true; + expect(livechatDepartmentStub.removeDepartmentFromUnit.calledOnceWith('department-id')).to.be.true; + expect(livechatUnitStub.decrementDepartmentsCount.calledOnceWith('unit-id')).to.be.true; + }); +}); From eb719860a62d24d75372b6170d34f4f7ab012d5d Mon Sep 17 00:00:00 2001 From: matheusbsilva137 Date: Thu, 4 Jul 2024 15:42:49 -0300 Subject: [PATCH 19/23] improve: Do not perform any action when an invalid unit id is provided --- .../server/hooks/manageDepartmentUnit.ts | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/apps/meteor/ee/app/livechat-enterprise/server/hooks/manageDepartmentUnit.ts b/apps/meteor/ee/app/livechat-enterprise/server/hooks/manageDepartmentUnit.ts index ac94d310db0b..7de7ef0d6bf6 100644 --- a/apps/meteor/ee/app/livechat-enterprise/server/hooks/manageDepartmentUnit.ts +++ b/apps/meteor/ee/app/livechat-enterprise/server/hooks/manageDepartmentUnit.ts @@ -25,10 +25,6 @@ export const manageDepartmentUnit = async ({ userId, departmentId, unitId }: { u return; } - if (currentDepartmentUnitId) { - await LivechatUnit.decrementDepartmentsCount(currentDepartmentUnitId); - } - if (unitId) { const unit = await LivechatUnit.findOneById>(unitId, { projection: { ancestors: 1 }, @@ -38,11 +34,19 @@ export const manageDepartmentUnit = async ({ userId, departmentId, unitId }: { u return; } + if (currentDepartmentUnitId) { + await LivechatUnit.decrementDepartmentsCount(currentDepartmentUnitId); + } + await LivechatDepartment.addDepartmentToUnit(departmentId, unitId, [unitId, ...(unit.ancestors || [])]); await LivechatUnit.incrementDepartmentsCount(unitId); return; } + if (currentDepartmentUnitId) { + await LivechatUnit.decrementDepartmentsCount(currentDepartmentUnitId); + } + await LivechatDepartment.removeDepartmentFromUnit(departmentId); }; From a6d0ec379624136ecd5a3ea461dd2ec7724464aa Mon Sep 17 00:00:00 2001 From: matheusbsilva137 Date: Thu, 4 Jul 2024 15:43:11 -0300 Subject: [PATCH 20/23] tests: improve unit tests --- .../server/hooks/manageDepartmentUnit.spec.ts | 23 ++++++++++++++++--- 1 file changed, 20 insertions(+), 3 deletions(-) diff --git a/apps/meteor/ee/tests/unit/apps/livechat-enterprise/server/hooks/manageDepartmentUnit.spec.ts b/apps/meteor/ee/tests/unit/apps/livechat-enterprise/server/hooks/manageDepartmentUnit.spec.ts index 3bc485a94902..8fbf0dcf97a2 100644 --- a/apps/meteor/ee/tests/unit/apps/livechat-enterprise/server/hooks/manageDepartmentUnit.spec.ts +++ b/apps/meteor/ee/tests/unit/apps/livechat-enterprise/server/hooks/manageDepartmentUnit.spec.ts @@ -92,6 +92,19 @@ describe('hooks/manageDepartmentUnit', () => { expect(livechatUnitStub.incrementDepartmentsCount.notCalled).to.be.true; }); + it('should not perform any operation if user is an admin/manager but an invalid unit is provided', async () => { + livechatDepartmentStub.findOneById.resolves({ _id: 'department-id', ancestors: ['unit-id'], parentId: 'unit-id' }); + livechatUnitStub.findOneById.resolves(undefined); + hasAnyRoleStub.resolves(true); + getUnitsFromUserStub.resolves(undefined); + + await manageDepartmentUnit({ userId: 'user-id', departmentId: 'department-id', unitId: 'new-unit-id' }); + expect(livechatDepartmentStub.addDepartmentToUnit.notCalled).to.be.true; + expect(livechatDepartmentStub.removeDepartmentFromUnit.notCalled).to.be.true; + expect(livechatUnitStub.decrementDepartmentsCount.notCalled).to.be.true; + expect(livechatUnitStub.incrementDepartmentsCount.notCalled).to.be.true; + }); + it('should remove department from its current unit if user is an admin/manager', async () => { livechatDepartmentStub.findOneById.resolves({ _id: 'department-id', ancestors: ['unit-id'], parentId: 'unit-id' }); hasAnyRoleStub.resolves(true); @@ -106,6 +119,7 @@ describe('hooks/manageDepartmentUnit', () => { it('should add department to a unit if user is an admin/manager', async () => { livechatDepartmentStub.findOneById.resolves({ _id: 'department-id' }); + livechatUnitStub.findOneById.resolves({ _id: 'unit-id' }); hasAnyRoleStub.resolves(true); getUnitsFromUserStub.resolves(undefined); @@ -118,13 +132,14 @@ describe('hooks/manageDepartmentUnit', () => { it('should move department to a new unit if user is an admin/manager', async () => { livechatDepartmentStub.findOneById.resolves({ _id: 'department-id', ancestors: ['unit-id'], parentId: 'unit-id' }); + livechatUnitStub.findOneById.resolves({ _id: 'new-unit-id' }); hasAnyRoleStub.resolves(true); getUnitsFromUserStub.resolves(undefined); await manageDepartmentUnit({ userId: 'user-id', departmentId: 'department-id', unitId: 'new-unit-id' }); expect(livechatDepartmentStub.addDepartmentToUnit.calledOnceWith('department-id', 'new-unit-id', ['new-unit-id'])).to.be.true; expect(livechatUnitStub.incrementDepartmentsCount.calledOnceWith('new-unit-id')).to.be.true; - expect(livechatDepartmentStub.removeDepartmentFromUnit.calledOnceWith('department-id')).to.be.true; + expect(livechatDepartmentStub.removeDepartmentFromUnit.notCalled).to.be.true; expect(livechatUnitStub.decrementDepartmentsCount.calledOnceWith('unit-id')).to.be.true; }); @@ -142,6 +157,7 @@ describe('hooks/manageDepartmentUnit', () => { it('should add department to a unit if user is a monitor that supervises the new unit', async () => { livechatDepartmentStub.findOneById.resolves({ _id: 'department-id' }); + livechatUnitStub.findOneById.resolves({ _id: 'unit-id' }); hasAnyRoleStub.resolves(false); getUnitsFromUserStub.resolves(['unit-id']); @@ -154,13 +170,14 @@ describe('hooks/manageDepartmentUnit', () => { it('should move department to a new unit if user is a monitor that supervises the current and new units', async () => { livechatDepartmentStub.findOneById.resolves({ _id: 'department-id', ancestors: ['unit-id'], parentId: 'unit-id' }); + livechatUnitStub.findOneById.resolves({ _id: 'unit-id' }); hasAnyRoleStub.resolves(false); - getUnitsFromUserStub.resolves(['unit-id', 'new-unit']); + getUnitsFromUserStub.resolves(['unit-id', 'new-unit-id']); await manageDepartmentUnit({ userId: 'user-id', departmentId: 'department-id', unitId: 'new-unit-id' }); expect(livechatDepartmentStub.addDepartmentToUnit.calledOnceWith('department-id', 'new-unit-id', ['new-unit-id'])).to.be.true; expect(livechatUnitStub.incrementDepartmentsCount.calledOnceWith('new-unit-id')).to.be.true; - expect(livechatDepartmentStub.removeDepartmentFromUnit.calledOnceWith('department-id')).to.be.true; + expect(livechatDepartmentStub.removeDepartmentFromUnit.notCalled).to.be.true; expect(livechatUnitStub.decrementDepartmentsCount.calledOnceWith('unit-id')).to.be.true; }); }); From 8673b91904417ac0581cb6428d76b15c9e6c6d6b Mon Sep 17 00:00:00 2001 From: matheusbsilva137 Date: Thu, 4 Jul 2024 15:45:09 -0300 Subject: [PATCH 21/23] tests: Fix tests descriptions (using admin instead of livechat manager) --- .../meteor/tests/end-to-end/api/livechat/14-units.ts | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/apps/meteor/tests/end-to-end/api/livechat/14-units.ts b/apps/meteor/tests/end-to-end/api/livechat/14-units.ts index 5123ad7f6c26..9ab644b9aee4 100644 --- a/apps/meteor/tests/end-to-end/api/livechat/14-units.ts +++ b/apps/meteor/tests/end-to-end/api/livechat/14-units.ts @@ -463,7 +463,7 @@ import { IS_EE } from '../../../e2e/config/constants'; await deleteDepartment(department._id); }); - it('should succesfully create a department into an existing unit as a livechat manager', async () => { + it('should succesfully create a department into an existing unit as an admin', async () => { const department = await createDepartment(undefined, undefined, undefined, undefined, { _id: unit._id }); const updatedUnit = await getUnit(unit._id); @@ -552,7 +552,7 @@ import { IS_EE } from '../../../e2e/config/constants'; }); }); - it('should succesfully add an existing department to a unit as a livechat manager', async () => { + it('should succesfully add an existing department to a unit as an admin', async () => { const updatedName = 'updated-department-name'; const updatedDepartment = await updateDepartment({ @@ -576,7 +576,7 @@ import { IS_EE } from '../../../e2e/config/constants'; expect(fullDepartment.ancestors?.[0]).to.equal(unit._id); }); - it('should succesfully remove an existing department from a unit as a livechat manager', async () => { + it('should succesfully remove an existing department from a unit as an admin', async () => { const updatedName = 'updated-department-name'; const updatedDepartment = await updateDepartment({ @@ -769,7 +769,7 @@ import { IS_EE } from '../../../e2e/config/constants'; await deleteDepartment(testDepartmentId); }); - it('should succesfully create a department into an existing unit as a livechat manager', async () => { + it('should succesfully create a department into an existing unit as an admin', async () => { const testDepartment = await createDepartmentWithMethod({ name: departmentName, departmentUnit: { _id: unit._id } }); testDepartmentId = testDepartment._id; @@ -784,7 +784,7 @@ import { IS_EE } from '../../../e2e/config/constants'; expect(fullDepartment.ancestors?.[0]).to.equal(unit._id); }); - it('should succesfully remove an existing department from a unit as a livechat manager', async () => { + it('should succesfully remove an existing department from a unit as an admin', async () => { await createDepartmentWithMethod({ name: departmentName, departmentUnit: {}, departmentId: testDepartmentId }); const updatedUnit = await getUnit(unit._id); @@ -797,7 +797,7 @@ import { IS_EE } from '../../../e2e/config/constants'; expect(fullDepartment).to.have.property('ancestors').that.is.null; }); - it('should succesfully add an existing department to a unit as a livechat manager', async () => { + it('should succesfully add an existing department to a unit as an admin', async () => { await createDepartmentWithMethod({ name: departmentName, departmentUnit: { _id: unit._id }, departmentId: testDepartmentId }); const updatedUnit = await getUnit(unit._id); From be49a844a23aef411c7ccc6568ca63157a9e5cec Mon Sep 17 00:00:00 2001 From: matheusbsilva137 Date: Mon, 29 Jul 2024 14:34:23 -0300 Subject: [PATCH 22/23] Throw error when trying to remove the last department from a unit --- .../app/livechat/server/lib/LivechatTyped.ts | 13 ++- .../server/models/raw/LivechatDepartment.ts | 4 + .../tests/end-to-end/api/livechat/14-units.ts | 102 ++++++++++++++---- .../core-typings/src/ILivechatDepartment.ts | 1 + .../src/models/ILivechatDepartmentModel.ts | 1 + 5 files changed, 102 insertions(+), 19 deletions(-) diff --git a/apps/meteor/app/livechat/server/lib/LivechatTyped.ts b/apps/meteor/app/livechat/server/lib/LivechatTyped.ts index afe7fb42fe5c..15f0ca358160 100644 --- a/apps/meteor/app/livechat/server/lib/LivechatTyped.ts +++ b/apps/meteor/app/livechat/server/lib/LivechatTyped.ts @@ -1760,7 +1760,18 @@ class LivechatClass { }); } - const department = _id ? await LivechatDepartment.findOneById(_id, { projection: { _id: 1, archived: 1, enabled: 1 } }) : null; + const department = _id + ? await LivechatDepartment.findOneById(_id, { projection: { _id: 1, archived: 1, enabled: 1, parentId: 1 } }) + : null; + + if (departmentUnit && !departmentUnit._id && department && department.parentId) { + const isLastDepartmentInUnit = (await LivechatDepartment.countDepartmentsInUnit(department.parentId)) === 1; + if (isLastDepartmentInUnit) { + throw new Meteor.Error('error-unit-cant-be-empty', "The last department in a unit can't be removed", { + method: 'livechat:saveDepartment', + }); + } + } if (!department && !(await isDepartmentCreationAvailable())) { throw new Meteor.Error('error-max-departments-number-reached', 'Maximum number of departments reached', { diff --git a/apps/meteor/server/models/raw/LivechatDepartment.ts b/apps/meteor/server/models/raw/LivechatDepartment.ts index 0dea973785cf..9ecee34df5e9 100644 --- a/apps/meteor/server/models/raw/LivechatDepartment.ts +++ b/apps/meteor/server/models/raw/LivechatDepartment.ts @@ -336,6 +336,10 @@ export class LivechatDepartmentRaw extends BaseRaw implemen return this.find(query, options); } + countDepartmentsInUnit(unitId: string): Promise { + return this.countDocuments({ parentId: unitId }); + } + findActiveByUnitIds(unitIds: string[], options: FindOptions = {}): FindCursor { const query = { enabled: true, diff --git a/apps/meteor/tests/end-to-end/api/livechat/14-units.ts b/apps/meteor/tests/end-to-end/api/livechat/14-units.ts index 9ab644b9aee4..e0c079ece243 100644 --- a/apps/meteor/tests/end-to-end/api/livechat/14-units.ts +++ b/apps/meteor/tests/end-to-end/api/livechat/14-units.ts @@ -504,6 +504,7 @@ import { IS_EE } from '../../../e2e/config/constants'; let monitor2Credentials: Awaited>; let unit: IOmnichannelBusinessUnit; let department: ILivechatDepartment; + let baseDepartment: ILivechatDepartment; before(async () => { monitor1 = await createUser(); @@ -513,10 +514,19 @@ import { IS_EE } from '../../../e2e/config/constants'; await createMonitor(monitor2.username); monitor2Credentials = await login(monitor2.username, password); department = await createDepartment(); - unit = await createUnit(monitor1._id, monitor1.username, []); + baseDepartment = await createDepartment(); + unit = await createUnit(monitor1._id, monitor1.username, [baseDepartment._id]); }); - after(async () => Promise.all([deleteUser(monitor1), deleteUser(monitor2), deleteUnit(unit), deleteDepartment(department._id)])); + after(async () => + Promise.all([ + deleteUser(monitor1), + deleteUser(monitor2), + deleteUnit(unit), + deleteDepartment(department._id), + deleteDepartment(baseDepartment._id), + ]), + ); it("should fail updating a department's unit when providing an invalid property in the department unit object", () => { const updatedName = 'updated-department-name'; @@ -552,6 +562,23 @@ import { IS_EE } from '../../../e2e/config/constants'; }); }); + it('should fail removing the last department from a unit', () => { + const updatedName = 'updated-department-name'; + return request + .put(api(`livechat/department/${baseDepartment._id}`)) + .set(credentials) + .send({ + department: { name: updatedName, enabled: true, showOnOfflineForm: true, showOnRegistration: true, email: 'bla@bla' }, + departmentUnit: {}, + }) + .expect('Content-Type', 'application/json') + .expect(400) + .expect((res) => { + expect(res.body).to.have.property('success', false); + expect(res.body).to.have.property('errorType', 'error-unit-cant-be-empty'); + }); + }); + it('should succesfully add an existing department to a unit as an admin', async () => { const updatedName = 'updated-department-name'; @@ -568,7 +595,7 @@ import { IS_EE } from '../../../e2e/config/constants'; const updatedUnit = await getUnit(unit._id); expect(updatedUnit).to.have.property('name', unit.name); expect(updatedUnit).to.have.property('numMonitors', 1); - expect(updatedUnit).to.have.property('numDepartments', 1); + expect(updatedUnit).to.have.property('numDepartments', 2); const fullDepartment = await getDepartmentById(department._id); expect(fullDepartment).to.have.property('parentId', unit._id); @@ -592,7 +619,7 @@ import { IS_EE } from '../../../e2e/config/constants'; const updatedUnit = await getUnit(unit._id); expect(updatedUnit).to.have.property('name', unit.name); expect(updatedUnit).to.have.property('numMonitors', 1); - expect(updatedUnit).to.have.property('numDepartments', 0); + expect(updatedUnit).to.have.property('numDepartments', 1); const fullDepartment = await getDepartmentById(department._id); expect(fullDepartment).to.have.property('parentId').that.is.null; @@ -615,7 +642,7 @@ import { IS_EE } from '../../../e2e/config/constants'; const updatedUnit = await getUnit(unit._id); expect(updatedUnit).to.have.property('name', unit.name); expect(updatedUnit).to.have.property('numMonitors', 1); - expect(updatedUnit).to.have.property('numDepartments', 0); + expect(updatedUnit).to.have.property('numDepartments', 1); const fullDepartment = await getDepartmentById(department._id); expect(fullDepartment).to.have.property('parentId').that.is.null; @@ -638,7 +665,7 @@ import { IS_EE } from '../../../e2e/config/constants'; const updatedUnit = await getUnit(unit._id); expect(updatedUnit).to.have.property('name', unit.name); expect(updatedUnit).to.have.property('numMonitors', 1); - expect(updatedUnit).to.have.property('numDepartments', 1); + expect(updatedUnit).to.have.property('numDepartments', 2); const fullDepartment = await getDepartmentById(department._id); expect(fullDepartment).to.have.property('name', updatedName); @@ -663,7 +690,7 @@ import { IS_EE } from '../../../e2e/config/constants'; const updatedUnit = await getUnit(unit._id); expect(updatedUnit).to.have.property('name', unit.name); expect(updatedUnit).to.have.property('numMonitors', 1); - expect(updatedUnit).to.have.property('numDepartments', 1); + expect(updatedUnit).to.have.property('numDepartments', 2); const fullDepartment = await getDepartmentById(department._id); expect(fullDepartment).to.have.property('name', updatedName); @@ -688,7 +715,7 @@ import { IS_EE } from '../../../e2e/config/constants'; const updatedUnit = await getUnit(unit._id); expect(updatedUnit).to.have.property('name', unit.name); expect(updatedUnit).to.have.property('numMonitors', 1); - expect(updatedUnit).to.have.property('numDepartments', 0); + expect(updatedUnit).to.have.property('numDepartments', 1); const fullDepartment = await getDepartmentById(department._id); expect(fullDepartment).to.have.property('name', updatedName); @@ -705,6 +732,7 @@ import { IS_EE } from '../../../e2e/config/constants'; let unit: IOmnichannelBusinessUnit; const departmentName = 'Test-Department-Livechat-Method'; let testDepartmentId = ''; + let baseDepartment: ILivechatDepartment; before(async () => { monitor1 = await createUser(); @@ -713,10 +741,19 @@ import { IS_EE } from '../../../e2e/config/constants'; monitor1Credentials = await login(monitor1.username, password); await createMonitor(monitor2.username); monitor2Credentials = await login(monitor2.username, password); - unit = await createUnit(monitor1._id, monitor1.username, []); + baseDepartment = await createDepartment(); + unit = await createUnit(monitor1._id, monitor1.username, [baseDepartment._id]); }); - after(async () => Promise.all([deleteUser(monitor1), deleteUser(monitor2), deleteUnit(unit), deleteDepartment(testDepartmentId)])); + after(async () => + Promise.all([ + deleteUser(monitor1), + deleteUser(monitor2), + deleteUnit(unit), + deleteDepartment(testDepartmentId), + deleteDepartment(baseDepartment._id), + ]), + ); it('should fail creating department when providing an invalid _id type in the department unit object', () => { return request @@ -747,6 +784,35 @@ import { IS_EE } from '../../../e2e/config/constants'; }); }); + it('should fail removing last department from unit', () => { + return request + .post(methodCall('livechat:saveDepartment')) + .set(credentials) + .send({ + message: JSON.stringify({ + method: 'livechat:saveDepartment', + params: [ + baseDepartment._id, + { name: 'Fail-Test', enabled: true, showOnOfflineForm: true, showOnRegistration: true, email: 'bla@bla' }, + [], + {}, + ], + id: 'id', + msg: 'method', + }), + }) + .expect('Content-Type', 'application/json') + .expect(200) + .expect((res) => { + expect(res.body).to.have.property('success', true); + expect(res.body).to.have.property('message').that.is.a('string'); + const data = JSON.parse(res.body.message); + expect(data).to.have.property('error').that.is.an('object'); + expect(data.error).to.have.property('errorType', 'Meteor.Error'); + expect(data.error).to.have.property('error', 'error-unit-cant-be-empty'); + }); + }); + it('should fail creating a department into an existing unit that a monitor does not supervise', async () => { const departmentName = 'Fail-Test'; @@ -760,7 +826,7 @@ import { IS_EE } from '../../../e2e/config/constants'; const updatedUnit = await getUnit(unit._id); expect(updatedUnit).to.have.property('name', unit.name); expect(updatedUnit).to.have.property('numMonitors', 1); - expect(updatedUnit).to.have.property('numDepartments', 0); + expect(updatedUnit).to.have.property('numDepartments', 1); const fullDepartment = await getDepartmentById(testDepartmentId); expect(fullDepartment).to.not.have.property('parentId'); @@ -776,7 +842,7 @@ import { IS_EE } from '../../../e2e/config/constants'; const updatedUnit = await getUnit(unit._id); expect(updatedUnit).to.have.property('name', unit.name); expect(updatedUnit).to.have.property('numMonitors', 1); - expect(updatedUnit).to.have.property('numDepartments', 1); + expect(updatedUnit).to.have.property('numDepartments', 2); const fullDepartment = await getDepartmentById(testDepartmentId); expect(fullDepartment).to.have.property('parentId', unit._id); @@ -790,7 +856,7 @@ import { IS_EE } from '../../../e2e/config/constants'; const updatedUnit = await getUnit(unit._id); expect(updatedUnit).to.have.property('name', unit.name); expect(updatedUnit).to.have.property('numMonitors', 1); - expect(updatedUnit).to.have.property('numDepartments', 0); + expect(updatedUnit).to.have.property('numDepartments', 1); const fullDepartment = await getDepartmentById(testDepartmentId); expect(fullDepartment).to.have.property('parentId').that.is.null; @@ -803,7 +869,7 @@ import { IS_EE } from '../../../e2e/config/constants'; const updatedUnit = await getUnit(unit._id); expect(updatedUnit).to.have.property('name', unit.name); expect(updatedUnit).to.have.property('numMonitors', 1); - expect(updatedUnit).to.have.property('numDepartments', 1); + expect(updatedUnit).to.have.property('numDepartments', 2); const fullDepartment = await getDepartmentById(testDepartmentId); expect(fullDepartment).to.have.property('parentId', unit._id); @@ -822,7 +888,7 @@ import { IS_EE } from '../../../e2e/config/constants'; const updatedUnit = await getUnit(unit._id); expect(updatedUnit).to.have.property('name', unit.name); expect(updatedUnit).to.have.property('numMonitors', 1); - expect(updatedUnit).to.have.property('numDepartments', 0); + expect(updatedUnit).to.have.property('numDepartments', 1); const fullDepartment = await getDepartmentById(testDepartmentId); expect(fullDepartment).to.have.property('parentId').that.is.null; @@ -840,7 +906,7 @@ import { IS_EE } from '../../../e2e/config/constants'; const updatedUnit = await getUnit(unit._id); expect(updatedUnit).to.have.property('name', unit.name); expect(updatedUnit).to.have.property('numMonitors', 1); - expect(updatedUnit).to.have.property('numDepartments', 1); + expect(updatedUnit).to.have.property('numDepartments', 2); const fullDepartment = await getDepartmentById(testDepartmentId); expect(fullDepartment).to.have.property('parentId', unit._id); @@ -859,7 +925,7 @@ import { IS_EE } from '../../../e2e/config/constants'; const updatedUnit = await getUnit(unit._id); expect(updatedUnit).to.have.property('name', unit.name); expect(updatedUnit).to.have.property('numMonitors', 1); - expect(updatedUnit).to.have.property('numDepartments', 1); + expect(updatedUnit).to.have.property('numDepartments', 2); const fullDepartment = await getDepartmentById(testDepartmentId); expect(fullDepartment).to.have.property('parentId', unit._id); @@ -881,7 +947,7 @@ import { IS_EE } from '../../../e2e/config/constants'; const updatedUnit = await getUnit(unit._id); expect(updatedUnit).to.have.property('name', unit.name); expect(updatedUnit).to.have.property('numMonitors', 1); - expect(updatedUnit).to.have.property('numDepartments', 2); + expect(updatedUnit).to.have.property('numDepartments', 3); const fullDepartment = await getDepartmentById(testDepartmentId); expect(fullDepartment).to.have.property('parentId', unit._id); diff --git a/packages/core-typings/src/ILivechatDepartment.ts b/packages/core-typings/src/ILivechatDepartment.ts index a73cf55cb235..0138a88226fb 100644 --- a/packages/core-typings/src/ILivechatDepartment.ts +++ b/packages/core-typings/src/ILivechatDepartment.ts @@ -16,6 +16,7 @@ export interface ILivechatDepartment { archived?: boolean; departmentsAllowedToForward?: string[]; maxNumberSimultaneousChat?: number; + parentId?: string; ancestors?: string[]; allowReceiveForwardOffline?: boolean; // extra optional fields diff --git a/packages/model-typings/src/models/ILivechatDepartmentModel.ts b/packages/model-typings/src/models/ILivechatDepartmentModel.ts index bf3ea879f607..fe366256eff7 100644 --- a/packages/model-typings/src/models/ILivechatDepartmentModel.ts +++ b/packages/model-typings/src/models/ILivechatDepartmentModel.ts @@ -59,6 +59,7 @@ export interface ILivechatDepartmentModel extends IBaseModel>; findOneByIdOrName(_idOrName: string, options?: FindOptions): Promise; findByUnitIds(unitIds: string[], options?: FindOptions): FindCursor; + countDepartmentsInUnit(unitId: string): Promise; findActiveByUnitIds(unitIds: string[], options?: FindOptions): FindCursor; findNotArchived(options?: FindOptions): FindCursor; getBusinessHoursWithDepartmentStatuses(): Promise< From 76d252c417a86ed167d7737caa694acf3363d816 Mon Sep 17 00:00:00 2001 From: matheusbsilva137 Date: Mon, 12 Aug 2024 21:17:18 -0300 Subject: [PATCH 23/23] Add missing export --- apps/meteor/tests/data/livechat/department.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/meteor/tests/data/livechat/department.ts b/apps/meteor/tests/data/livechat/department.ts index e283c4b9819b..d7f22fca970b 100644 --- a/apps/meteor/tests/data/livechat/department.ts +++ b/apps/meteor/tests/data/livechat/department.ts @@ -42,7 +42,7 @@ const updateDepartment = async (departmentId: string, departmentData: Partial