From f8244ca98d8382e74dcee4531058155925f93bc1 Mon Sep 17 00:00:00 2001 From: rocketchat-github-ci Date: Wed, 20 Mar 2024 19:47:39 +0000 Subject: [PATCH 1/4] Bump 6.6.5 --- .changeset/bump-patch-1710964059521.md | 5 +++++ yarn.lock | 16 ++++++++-------- 2 files changed, 13 insertions(+), 8 deletions(-) create mode 100644 .changeset/bump-patch-1710964059521.md diff --git a/.changeset/bump-patch-1710964059521.md b/.changeset/bump-patch-1710964059521.md new file mode 100644 index 0000000000000..e1eaa7980afb1 --- /dev/null +++ b/.changeset/bump-patch-1710964059521.md @@ -0,0 +1,5 @@ +--- +'@rocket.chat/meteor': patch +--- + +Bump @rocket.chat/meteor version. diff --git a/yarn.lock b/yarn.lock index 8e65c735f6b7f..ad4b44b6d3d53 100644 --- a/yarn.lock +++ b/yarn.lock @@ -9348,9 +9348,9 @@ __metadata: "@rocket.chat/icons": "*" "@rocket.chat/prettier-config": "*" "@rocket.chat/styled": "*" - "@rocket.chat/ui-contexts": 4.0.3 + "@rocket.chat/ui-contexts": 4.0.4 "@rocket.chat/ui-kit": 0.33.0 - "@rocket.chat/ui-video-conf": 4.0.3 + "@rocket.chat/ui-video-conf": 4.0.4 "@tanstack/react-query": "*" react: "*" react-dom: "*" @@ -9432,14 +9432,14 @@ __metadata: ts-jest: ~29.1.1 typescript: ~5.3.2 peerDependencies: - "@rocket.chat/core-typings": 6.6.3 + "@rocket.chat/core-typings": 6.6.4 "@rocket.chat/css-in-js": "*" "@rocket.chat/fuselage": "*" "@rocket.chat/fuselage-tokens": "*" "@rocket.chat/message-parser": "*" "@rocket.chat/styled": "*" - "@rocket.chat/ui-client": 4.0.3 - "@rocket.chat/ui-contexts": 4.0.3 + "@rocket.chat/ui-client": 4.0.4 + "@rocket.chat/ui-contexts": 4.0.4 katex: "*" react: "*" languageName: unknown @@ -10621,7 +10621,7 @@ __metadata: "@rocket.chat/fuselage": "*" "@rocket.chat/fuselage-hooks": "*" "@rocket.chat/icons": "*" - "@rocket.chat/ui-contexts": 4.0.3 + "@rocket.chat/ui-contexts": 4.0.4 react: ~17.0.2 languageName: unknown linkType: soft @@ -10796,7 +10796,7 @@ __metadata: "@rocket.chat/fuselage-hooks": "*" "@rocket.chat/icons": "*" "@rocket.chat/styled": "*" - "@rocket.chat/ui-contexts": 4.0.3 + "@rocket.chat/ui-contexts": 4.0.4 react: ^17.0.2 react-dom: ^17.0.2 languageName: unknown @@ -10885,7 +10885,7 @@ __metadata: peerDependencies: "@rocket.chat/layout": "*" "@rocket.chat/tools": 0.2.1 - "@rocket.chat/ui-contexts": 4.0.3 + "@rocket.chat/ui-contexts": 4.0.4 "@tanstack/react-query": "*" react: "*" react-hook-form: "*" From 61814956216f08affee2d60269e12469275f7919 Mon Sep 17 00:00:00 2001 From: Kevin Aleman Date: Thu, 21 Mar 2024 07:15:40 -0600 Subject: [PATCH 2/4] fix: Allow queue to fix inquiry status when processing items (#31998) Co-authored-by: Marcos Spessatto Defendi --- .changeset/tough-doors-juggle.md | 9 + .../server/services/omnichannel/queue.ts | 77 ++- .../services/omnichannel/queue.tests.ts | 475 ++++++++++++++++++ 3 files changed, 552 insertions(+), 9 deletions(-) create mode 100644 .changeset/tough-doors-juggle.md create mode 100644 apps/meteor/tests/unit/server/services/omnichannel/queue.tests.ts diff --git a/.changeset/tough-doors-juggle.md b/.changeset/tough-doors-juggle.md new file mode 100644 index 0000000000000..9da2ab1c60753 --- /dev/null +++ b/.changeset/tough-doors-juggle.md @@ -0,0 +1,9 @@ +--- +"@rocket.chat/meteor": patch +--- + +Introduced a new step to the queue worker: when an inquiry that's on an improper status is selected for processing, queue worker will first check its status and will attempt to fix it. +For example, if an inquiry points to a closed room, there's no point in processing, system will now remove the inquiry +If an inquiry is already taken, the inquiry will be updated to reflect the new status and clean the queue. + +This prevents issues where the queue worker attempted to process an inquiry _forever_ because it was in an improper state. diff --git a/apps/meteor/server/services/omnichannel/queue.ts b/apps/meteor/server/services/omnichannel/queue.ts index 603c5197ed30b..0a9a13cffad99 100644 --- a/apps/meteor/server/services/omnichannel/queue.ts +++ b/apps/meteor/server/services/omnichannel/queue.ts @@ -1,6 +1,7 @@ -import type { InquiryWithAgentInfo, IOmnichannelQueue } from '@rocket.chat/core-typings'; +import type { IOmnichannelRoom } from '@rocket.chat/core-typings'; +import { type InquiryWithAgentInfo, type IOmnichannelQueue } from '@rocket.chat/core-typings'; import { License } from '@rocket.chat/license'; -import { LivechatInquiry } from '@rocket.chat/models'; +import { LivechatInquiry, LivechatRooms } from '@rocket.chat/models'; import { dispatchAgentDelegated } from '../../../app/livechat/server/lib/Helper'; import { RoutingManager } from '../../../app/livechat/server/lib/RoutingManager'; @@ -92,9 +93,11 @@ export class OmnichannelQueue implements IOmnichannelQueue { // Note: this removes the "one-shot" behavior of queue, allowing it to take a conversation again in the future // And sorting them by _updatedAt: -1 will make it so that the oldest inquiries are taken first // preventing us from playing with the same inquiry over and over again + queueLogger.debug(`Inquiry ${nextInquiry._id} not taken. Unlocking and re-queueing`); return await LivechatInquiry.unlockAndQueue(nextInquiry._id); } + queueLogger.debug(`Inquiry ${nextInquiry._id} taken successfully. Unlocking`); await LivechatInquiry.unlock(nextInquiry._id); } catch (e) { queueLogger.error({ @@ -123,17 +126,74 @@ export class OmnichannelQueue implements IOmnichannelQueue { void (routingSupportsAutoAssign ? this.start() : this.stop()); } + private async reconciliation(reason: 'closed' | 'taken' | 'missing', { roomId, inquiryId }: { roomId: string; inquiryId: string }) { + switch (reason) { + case 'closed': { + queueLogger.debug({ + msg: 'Room closed. Removing inquiry', + roomId, + inquiryId, + step: 'reconciliation', + }); + await LivechatInquiry.removeByRoomId(roomId); + break; + } + case 'taken': { + queueLogger.debug({ + msg: 'Room taken. Updating inquiry status', + roomId, + inquiryId, + step: 'reconciliation', + }); + // Reconciliate served inquiries, by updating their status to taken after queue tried to pick and failed + await LivechatInquiry.takeInquiry(inquiryId); + break; + } + case 'missing': { + queueLogger.debug({ + msg: 'Room from inquiry missing. Removing inquiry', + roomId, + inquiryId, + step: 'reconciliation', + }); + await LivechatInquiry.removeByRoomId(roomId); + break; + } + default: { + return true; + } + } + + return true; + } + private async processWaitingQueue(department: string | undefined, inquiry: InquiryWithAgentInfo) { const queue = department || 'Public'; - queueLogger.debug(`Processing items on queue ${queue}`); queueLogger.debug(`Processing inquiry ${inquiry._id} from queue ${queue}`); const { defaultAgent } = inquiry; - const room = await RoutingManager.delegateInquiry(inquiry, defaultAgent); - const propagateAgentDelegated = async (rid: string, agentId: string) => { - await dispatchAgentDelegated(rid, agentId); - }; + const roomFromDb = await LivechatRooms.findOneById>(inquiry.rid, { + projection: { servedBy: 1, closedAt: 1 }, + }); + + // This is a precaution to avoid taking inquiries tied to rooms that no longer exist. + // This should never happen. + if (!roomFromDb) { + return this.reconciliation('missing', { roomId: inquiry.rid, inquiryId: inquiry._id }); + } + + // This is a precaution to avoid taking the same inquiry multiple times. It should not happen, but it's a safety net + if (roomFromDb.servedBy) { + return this.reconciliation('taken', { roomId: inquiry.rid, inquiryId: inquiry._id }); + } + + // This is another precaution. If the room is closed, we should not take it + if (roomFromDb.closedAt) { + return this.reconciliation('closed', { roomId: inquiry.rid, inquiryId: inquiry._id }); + } + + const room = await RoutingManager.delegateInquiry(inquiry, defaultAgent); if (room?.servedBy) { const { @@ -142,13 +202,12 @@ export class OmnichannelQueue implements IOmnichannelQueue { } = room; queueLogger.debug(`Inquiry ${inquiry._id} taken successfully by agent ${agentId}. Notifying`); setTimeout(() => { - void propagateAgentDelegated(rid, agentId); + void dispatchAgentDelegated(rid, agentId); }, 1000); return true; } - queueLogger.debug(`Inquiry ${inquiry._id} not taken by any agent. Queueing again`); return false; } } diff --git a/apps/meteor/tests/unit/server/services/omnichannel/queue.tests.ts b/apps/meteor/tests/unit/server/services/omnichannel/queue.tests.ts new file mode 100644 index 0000000000000..ce24b0ded64a5 --- /dev/null +++ b/apps/meteor/tests/unit/server/services/omnichannel/queue.tests.ts @@ -0,0 +1,475 @@ +import { expect } from 'chai'; +import { beforeEach, describe, after, it } from 'mocha'; +import p from 'proxyquire'; +import Sinon from 'sinon'; + +const dispatchAgentDelegated = Sinon.stub(); +const getConfig = Sinon.stub(); +const delegateInquiry = Sinon.stub(); +const libSettings = { getInquirySortMechanismSetting: Sinon.stub().returns('timestamp') }; +const settings = { + get: Sinon.stub(), +}; + +const queueLogger = { + info: Sinon.stub(), + debug: Sinon.stub(), + error: Sinon.stub(), +}; + +const mockedInquiry = { + _id: 'inquiryId', + rid: 'rid', + department: 'department1', + ts: new Date(), +}; + +const models = { + LivechatInquiry: { + unlockAll: Sinon.stub(), + findNextAndLock: Sinon.stub(), + getDistinctQueuedDepartments: Sinon.stub(), + unlockAndQueue: Sinon.stub(), + unlock: Sinon.stub(), + removeByRoomId: Sinon.stub(), + takeInquiry: Sinon.stub(), + }, + LivechatRooms: { + findOneById: Sinon.stub(), + }, +}; + +const license = { + shouldPreventAction: Sinon.stub(), +}; + +const { OmnichannelQueue } = p.noCallThru().load('../../../../../server/services/omnichannel/queue', { + '../../../app/livechat/server/lib/Helper': { + dispatchAgentDelegated, + }, + '../../../app/livechat/server/lib/RoutingManager': { + RoutingManager: { + getConfig, + delegateInquiry, + }, + }, + '../../../app/livechat/server/lib/settings': libSettings, + '../../../app/settings/server': { settings }, + './logger': { queueLogger }, + '@rocket.chat/models': models, + '@rocket.chat/license': { License: license }, +}); + +describe('Omnichannel Queue processor', () => { + describe('isRunning', () => { + it('should return the running status', () => { + const queue = new OmnichannelQueue(); + expect(queue.isRunning()).to.be.false; + }); + it('should return the running status', () => { + const queue = new OmnichannelQueue(); + queue.running = true; + expect(queue.isRunning()).to.be.true; + }); + }); + describe('delay', () => { + after(() => { + settings.get.reset(); + }); + it('should return 5000 if setting is not set', () => { + settings.get.returns(undefined); + + const queue = new OmnichannelQueue(); + expect(queue.delay()).to.be.equal(5000); + }); + it('should return the right value if setting has a value above 1', () => { + settings.get.returns(10); + + const queue = new OmnichannelQueue(); + expect(queue.delay()).to.be.equal(10000); + }); + }); + describe('getActiveQueues', () => { + after(() => { + models.LivechatInquiry.getDistinctQueuedDepartments.reset(); + }); + it('should return [undefined] when there is no other queues', async () => { + models.LivechatInquiry.getDistinctQueuedDepartments.returns([]); + + const queue = new OmnichannelQueue(); + expect(await queue.getActiveQueues()).to.be.eql([undefined]); + }); + it('should return [undefined, department1] when department1 is an active queue', async () => { + models.LivechatInquiry.getDistinctQueuedDepartments.returns(['department1']); + + const queue = new OmnichannelQueue(); + expect(await queue.getActiveQueues()).to.be.eql([undefined, 'department1']); + }); + }); + describe('nextQueue', () => { + after(() => { + models.LivechatInquiry.getDistinctQueuedDepartments.reset(); + }); + it('should return undefined when thats the only queue', async () => { + models.LivechatInquiry.getDistinctQueuedDepartments.returns([]); + + const queue = new OmnichannelQueue(); + queue.getActiveQueues = Sinon.stub().returns([undefined]); + expect(await queue.nextQueue()).to.be.undefined; + }); + it('should return undefined, and then the following queue', async () => { + models.LivechatInquiry.getDistinctQueuedDepartments.returns(['department1']); + + const queue = new OmnichannelQueue(); + queue.getActiveQueues = Sinon.stub().returns([undefined, 'department1']); + expect(await queue.nextQueue()).to.be.undefined; + expect(await queue.nextQueue()).to.be.equal('department1'); + }); + it('should not call getActiveQueues if there are still queues to process', async () => { + models.LivechatInquiry.getDistinctQueuedDepartments.returns(['department1']); + + const queue = new OmnichannelQueue(); + queue.queues = ['department1']; + queue.getActiveQueues = Sinon.stub(); + + expect(await queue.nextQueue()).to.be.equal('department1'); + expect(queue.getActiveQueues.notCalled).to.be.true; + }); + }); + describe('checkQueue', () => { + let clock: any; + beforeEach(() => { + models.LivechatInquiry.findNextAndLock.resetHistory(); + models.LivechatInquiry.takeInquiry.resetHistory(); + models.LivechatInquiry.unlockAndQueue.resetHistory(); + models.LivechatInquiry.unlock.resetHistory(); + queueLogger.error.resetHistory(); + queueLogger.info.resetHistory(); + clock = Sinon.useFakeTimers(); + }); + afterEach(() => { + clock.restore(); + }); + after(() => { + models.LivechatInquiry.findNextAndLock.reset(); + models.LivechatInquiry.takeInquiry.reset(); + models.LivechatInquiry.unlockAndQueue.reset(); + models.LivechatInquiry.unlock.reset(); + queueLogger.error.reset(); + queueLogger.info.reset(); + clock.reset(); + }); + + it('should return undefined when the queue is empty', async () => { + models.LivechatInquiry.findNextAndLock.returns(null); + + const queue = new OmnichannelQueue(); + queue.execute = Sinon.stub(); + expect(await queue.checkQueue()).to.be.undefined; + }); + it('should try to process the inquiry when there is one', async () => { + models.LivechatInquiry.findNextAndLock.returns(mockedInquiry); + + const queue = new OmnichannelQueue(); + queue.processWaitingQueue = Sinon.stub().throws('error'); + queue.execute = Sinon.stub(); + await queue.checkQueue(); + + expect(models.LivechatInquiry.findNextAndLock.calledOnce).to.be.true; + expect(queue.processWaitingQueue.calledOnce).to.be.true; + }); + it('should call unlockAndRequeue when the inquiry could not be processed', async () => { + models.LivechatInquiry.findNextAndLock.returns(mockedInquiry); + + const queue = new OmnichannelQueue(); + queue.processWaitingQueue = Sinon.stub().returns(false); + queue.execute = Sinon.stub(); + await queue.checkQueue(); + + expect(queue.processWaitingQueue.calledOnce).to.be.true; + expect(models.LivechatInquiry.unlockAndQueue.calledOnce).to.be.true; + }); + it('should unlock the inquiry when it was processed succesfully', async () => { + models.LivechatInquiry.findNextAndLock.returns(mockedInquiry); + + const queue = new OmnichannelQueue(); + queue.processWaitingQueue = Sinon.stub().returns(true); + queue.execute = Sinon.stub(); + await queue.checkQueue(); + + expect(queue.processWaitingQueue.calledOnce).to.be.true; + expect(models.LivechatInquiry.unlock.calledOnce).to.be.true; + }); + it('should print a log when there was an error processing inquiry', async () => { + models.LivechatInquiry.findNextAndLock.throws('error'); + + const queue = new OmnichannelQueue(); + queue.execute = Sinon.stub(); + await queue.checkQueue(); + + expect(queueLogger.error.calledOnce).to.be.true; + }); + it('should call execute after finishing', async () => { + models.LivechatInquiry.findNextAndLock.returns(mockedInquiry); + + const queue = new OmnichannelQueue(); + queue.processWaitingQueue = Sinon.stub().returns(true); + queue.execute = Sinon.stub(); + queue.delay = Sinon.stub().returns(100); + await queue.checkQueue(); + clock.tick(100); + + expect(queue.execute.calledOnce).to.be.true; + expect(models.LivechatInquiry.unlock.calledOnce).to.be.true; + expect(queue.execute.calledAfter(models.LivechatInquiry.unlock)).to.be.true; + expect(queue.execute.calledOnce).to.be.true; + }); + }); + describe('shouldStart', () => { + beforeEach(() => { + settings.get.resetHistory(); + getConfig.resetHistory(); + }); + after(() => { + settings.get.reset(); + getConfig.reset(); + }); + + it('should call stop if Livechat is not enabled', async () => { + settings.get.returns(false); + + const queue = new OmnichannelQueue(); + queue.stop = Sinon.stub(); + await queue.shouldStart(); + + expect(queue.stop.calledOnce).to.be.true; + }); + it('should call start if routing algorithm supports auto assignment', async () => { + settings.get.returns(true); + getConfig.returns({ autoAssignAgent: true }); + + const queue = new OmnichannelQueue(); + queue.start = Sinon.stub(); + await queue.shouldStart(); + + expect(queue.start.calledOnce).to.be.true; + expect(queue.start.calledAfter(getConfig)).to.be.true; + }); + it('should call stop if routing algorithm does not support auto assignment', async () => { + settings.get.returns(true); + getConfig.returns({ autoAssignAgent: false }); + + const queue = new OmnichannelQueue(); + queue.stop = Sinon.stub(); + await queue.shouldStart(); + + expect(queue.stop.calledOnce).to.be.true; + expect(queue.stop.calledAfter(getConfig)).to.be.true; + }); + }); + describe('reconciliation', () => { + beforeEach(() => { + models.LivechatInquiry.removeByRoomId.resetHistory(); + models.LivechatInquiry.takeInquiry.resetHistory(); + }); + + it('should remove inquiries from rooms that do not exist', async () => { + const queue = new OmnichannelQueue(); + await queue.reconciliation('missing', { roomId: 'rid', inquiryId: 'inquiryId' }); + + expect(models.LivechatInquiry.removeByRoomId.calledOnce).to.be.true; + }); + it('should take an inquiry if the room was taken', async () => { + const queue = new OmnichannelQueue(); + await queue.reconciliation('taken', { roomId: 'rid', inquiryId: 'inquiryId' }); + + expect(models.LivechatInquiry.takeInquiry.calledOnce).to.be.true; + }); + it('should remove inquiries from rooms that were closed', async () => { + const queue = new OmnichannelQueue(); + await queue.reconciliation('closed', { roomId: 'rid', inquiryId: 'inquiryId' }); + + expect(models.LivechatInquiry.removeByRoomId.calledOnce).to.be.true; + }); + it('should return true for any other case', async () => { + const queue = new OmnichannelQueue(); + expect(await queue.reconciliation('random', { roomId: 'rid', inquiryId: 'inquiryId' })).to.be.true; + expect(models.LivechatInquiry.removeByRoomId.notCalled).to.be.true; + expect(models.LivechatInquiry.takeInquiry.notCalled).to.be.true; + }); + }); + describe('processWaitingQueue', () => { + let clock: any; + beforeEach(() => { + models.LivechatRooms.findOneById.reset(); + models.LivechatInquiry.takeInquiry.resetHistory(); + models.LivechatInquiry.removeByRoomId.resetHistory(); + delegateInquiry.resetHistory(); + queueLogger.debug.resetHistory(); + clock = Sinon.useFakeTimers(); + }); + afterEach(() => { + clock.restore(); + }); + after(() => { + models.LivechatRooms.findOneById.reset(); + models.LivechatInquiry.takeInquiry.reset(); + delegateInquiry.reset(); + queueLogger.debug.reset(); + clock.reset(); + }); + + it('should process the public queue when department is undefined', async () => { + const queue = new OmnichannelQueue(); + + expect(await queue.processWaitingQueue(undefined, mockedInquiry)).to.be.true; + expect(queueLogger.debug.calledWith('Processing inquiry inquiryId from queue Public')); + expect(models.LivechatRooms.findOneById.calledOnce).to.be.true; + }); + it('should call removeInquiry when findOneById returns null', async () => { + models.LivechatRooms.findOneById.returns(null); + + const queue = new OmnichannelQueue(); + expect(await queue.processWaitingQueue('department1', mockedInquiry)).to.be.true; + expect( + queueLogger.debug.calledWith({ + msg: 'Room from inquiry missing. Removing inquiry', + roomId: 'rid', + inquiryId: 'inquiryId', + step: 'reconciliation', + }), + ).to.be.true; + expect(models.LivechatInquiry.removeByRoomId.calledOnce).to.be.true; + }); + it('should call takeInquiry when findOneById returns a room thats already being served', async () => { + models.LivechatRooms.findOneById.returns({ _id: 'rid', servedBy: { some: 'thing' } }); + + const queue = new OmnichannelQueue(); + queue.reconciliation = Sinon.stub().returns(true); + expect(await queue.processWaitingQueue('department1', mockedInquiry)).to.be.true; + expect(queue.reconciliation.calledOnce).to.be.true; + }); + it('should call removeInquiry when findOneById returns a room that was closed', async () => { + models.LivechatRooms.findOneById.returns({ _id: 'rid', closedAt: new Date() }); + + const queue = new OmnichannelQueue(); + queue.reconciliation = Sinon.stub().returns(true); + expect(await queue.processWaitingQueue('department1', mockedInquiry)).to.be.true; + expect(queue.reconciliation.calledOnce).to.be.true; + }); + it('should call delegateInquiry when prechecks are met and return false if inquiry was not served', async () => { + models.LivechatRooms.findOneById.returns({ _id: 'rid' }); + delegateInquiry.returns({}); + + const queue = new OmnichannelQueue(); + expect(await queue.processWaitingQueue('department1', mockedInquiry)).to.be.false; + expect(delegateInquiry.calledOnce).to.be.true; + }); + it('should call delegateInquiry and return true if inquiry was served', async () => { + models.LivechatRooms.findOneById.returns({ _id: 'rid' }); + delegateInquiry.returns({ _id: 'rid', servedBy: { _id: 'agentId' } }); + + const queue = new OmnichannelQueue(); + expect(await queue.processWaitingQueue('department1', mockedInquiry)).to.be.true; + expect(delegateInquiry.calledOnce).to.be.true; + }); + it('should call dispatchAgentDelegated if inquiry was served (after 1s)', async () => { + models.LivechatRooms.findOneById.returns({ _id: 'rid' }); + delegateInquiry.returns({ _id: 'rid', servedBy: { _id: 'agentId' } }); + + const queue = new OmnichannelQueue(); + expect(await queue.processWaitingQueue('department1', mockedInquiry)).to.be.true; + expect(delegateInquiry.calledOnce).to.be.true; + clock.tick(1000); + expect(dispatchAgentDelegated.calledOnce).to.be.true; + }); + }); + describe('execute', () => { + beforeEach(() => { + license.shouldPreventAction.reset(); + queueLogger.debug.reset(); + }); + + after(() => { + license.shouldPreventAction.reset(); + queueLogger.debug.reset(); + }); + + it('should return undefined if service is not running', async () => { + const queue = new OmnichannelQueue(); + queue.running = false; + expect(await queue.execute()).to.be.undefined; + }); + it('should return undefined if license is over mac limits', async () => { + license.shouldPreventAction.returns(true); + + const queue = new OmnichannelQueue(); + queue.running = true; + expect(await queue.execute()).to.be.undefined; + expect(license.shouldPreventAction.calledOnce).to.be.true; + expect(queue.running).to.be.false; + }); + it('should try to process a queue if license is not over mac limits', async () => { + license.shouldPreventAction.returns(false); + + const queue = new OmnichannelQueue(); + queue.running = true; + queue.nextQueue = Sinon.stub(); + await queue.execute(); + + expect(queue.nextQueue.calledOnce).to.be.true; + expect(queueLogger.debug.calledWith('Executing queue Public with timeout of 5000')).to.be.true; + }); + }); + describe('start', () => { + beforeEach(() => { + queueLogger.info.resetHistory(); + queueLogger.debug.resetHistory(); + }); + after(() => { + queueLogger.info.reset(); + queueLogger.debug.reset(); + }); + it('should do nothing if queue is already running', async () => { + const queue = new OmnichannelQueue(); + queue.running = true; + queue.execute = Sinon.stub(); + await queue.start(); + + expect(queue.execute.notCalled).to.be.true; + }); + it('should fetch active queues and set running to true', async () => { + const queue = new OmnichannelQueue(); + queue.running = false; + queue.getActiveQueues = Sinon.stub().returns(['department1']); + queue.execute = Sinon.stub(); + await queue.start(); + + expect(queue.running).to.be.true; + expect(queue.getActiveQueues.calledOnce).to.be.true; + expect(queueLogger.info.calledOnce).to.be.true; + expect(queueLogger.info.calledWith('Service started')).to.be.true; + expect(queue.execute.calledOnce).to.be.true; + }); + }); + describe('stop', () => { + beforeEach(() => { + models.LivechatInquiry.unlockAll.reset(); + queueLogger.info.resetHistory(); + }); + after(() => { + models.LivechatInquiry.unlockAll.reset(); + queueLogger.info.reset(); + }); + it('should unlock all inquiries and set running to false', async () => { + const queue = new OmnichannelQueue(); + queue.running = true; + await queue.stop(); + + expect(queue.running).to.be.false; + expect(models.LivechatInquiry.unlockAll.calledOnce).to.be.true; + expect(queueLogger.info.calledOnce).to.be.true; + expect(queueLogger.info.calledWith('Service stopped')).to.be.true; + }); + }); +}); From 6476aa947c067007dde09aae65a17c52c94e74e7 Mon Sep 17 00:00:00 2001 From: Guilherme Gazzo Date: Fri, 8 Mar 2024 13:33:41 -0300 Subject: [PATCH 3/4] fix: livechat sdk reconnect not resubscribing (#31941) --- .changeset/empty-hounds-jog.md | 5 +++++ ee/packages/ddp-client/src/livechat/LivechatClientImpl.ts | 4 ++-- 2 files changed, 7 insertions(+), 2 deletions(-) create mode 100644 .changeset/empty-hounds-jog.md diff --git a/.changeset/empty-hounds-jog.md b/.changeset/empty-hounds-jog.md new file mode 100644 index 0000000000000..b6830e0522ef1 --- /dev/null +++ b/.changeset/empty-hounds-jog.md @@ -0,0 +1,5 @@ +--- +"@rocket.chat/ddp-client": patch +--- + +fix: livechat sdk reconnect not resubscribing diff --git a/ee/packages/ddp-client/src/livechat/LivechatClientImpl.ts b/ee/packages/ddp-client/src/livechat/LivechatClientImpl.ts index fa28d48ba5c8c..973853678f6e6 100644 --- a/ee/packages/ddp-client/src/livechat/LivechatClientImpl.ts +++ b/ee/packages/ddp-client/src/livechat/LivechatClientImpl.ts @@ -381,9 +381,9 @@ export class LivechatClientImpl extends DDPSDK implements LivechatStream, Livech const sdk = new LivechatClientImpl(connection, stream, account, timeoutControl, rest); connection.on('connected', () => { - Object.entries(stream.subscriptions).forEach(([, sub]) => { + for (const [, sub] of stream.subscriptions.entries()) { ddp.subscribeWithId(sub.id, sub.name, sub.params); - }); + } }); return sdk; From 1b1164189c04855fe30e391cbd7b28516c8fa133 Mon Sep 17 00:00:00 2001 From: Martin Schoeler Date: Mon, 11 Mar 2024 20:33:01 -0300 Subject: [PATCH 4/4] fix: setDepartment not persisting on chat closed (#31792) --- .changeset/thin-keys-impress.md | 5 + .../omnichannel-livechat-api.spec.ts | 93 ++++++++++++++----- packages/livechat/src/lib/hooks.js | 1 + packages/livechat/src/lib/room.js | 4 +- 4 files changed, 77 insertions(+), 26 deletions(-) create mode 100644 .changeset/thin-keys-impress.md diff --git a/.changeset/thin-keys-impress.md b/.changeset/thin-keys-impress.md new file mode 100644 index 0000000000000..8679299972fcc --- /dev/null +++ b/.changeset/thin-keys-impress.md @@ -0,0 +1,5 @@ +--- +"@rocket.chat/livechat": patch +--- + +Fixes issue of the `setDepartment` Livechat API method not setting the store value properly (is was only setting on the guest object) diff --git a/apps/meteor/tests/e2e/omnichannel/omnichannel-livechat-api.spec.ts b/apps/meteor/tests/e2e/omnichannel/omnichannel-livechat-api.spec.ts index c8810fa4eb779..a8fa27eb5d2c7 100644 --- a/apps/meteor/tests/e2e/omnichannel/omnichannel-livechat-api.spec.ts +++ b/apps/meteor/tests/e2e/omnichannel/omnichannel-livechat-api.spec.ts @@ -6,6 +6,7 @@ import { createAuxContext } from '../fixtures/createAuxContext'; import { Users } from '../fixtures/userStates'; import { HomeOmnichannel, OmnichannelLiveChatEmbedded } from '../page-objects'; import { createAgent } from '../utils/omnichannel/agents'; +import { addAgentToDepartment, createDepartment } from '../utils/omnichannel/departments'; import { test, expect } from '../utils/test'; // TODO: Use official widget typing once that is merged @@ -32,7 +33,7 @@ declare const window: Window & { setAgent: (agent: { username: string; _id: string }) => void; setBusinessUnit: (businessUnit?: string) => void; setCustomField: (field: { key: string; value: string }) => void; - setDepartment: (department: { _id: string; name: string }) => void; + setDepartment: (dep: string) => void; setGuestEmail: (email: string) => void; setGuestName: (name: string) => void; setGuestToken: (token: string) => void; @@ -56,8 +57,8 @@ declare const window: Window & { }; test.describe('OC - Livechat API', () => { - // TODO: Check if there is a way to add livechat to the global window object - + // TODO: Check if there is a way to add livechat to the global window object + test.describe('Basic Widget Interactions', () => { // Tests that rely only on the widget itself, without requiring further interaction from the main RC app let poAuxContext: { page: Page; poHomeOmnichannel: HomeOmnichannel }; @@ -213,27 +214,24 @@ test.describe('OC - Livechat API', () => { test.skip(!IS_EE, 'Enterprise Only'); // Tests that requires interaction from an agent or more let poAuxContext: { page: Page; poHomeOmnichannel: HomeOmnichannel }; + let poAuxContext2: { page: Page; poHomeOmnichannel: HomeOmnichannel }; let poLiveChat: OmnichannelLiveChatEmbedded; let page: Page; - let depId: string; let agent: Awaited>; + let agent2: Awaited>; + let departments: Awaited>[]; + test.beforeAll(async ({ api }) => { agent = await createAgent(api, 'user1') - - const response = await api.post('/livechat/department', {department: { - enabled: true, - email: faker.internet.email(), - showOnRegistration: true, - showOnOfflineForm: true, - name: `new department ${Date.now()}`, - description: 'created from api', - }}); - - expect(response.status()).toBe(200); - - const resBody = await response.json(); - depId = resBody.department._id; + agent2 = await createAgent(api, 'user2') + + departments = await Promise.all([createDepartment(api), createDepartment(api)]); + const [departmentA, departmentB] = departments.map(({ data }) => data); + + await addAgentToDepartment(api, { department: departmentA, agentId: agent.data._id }) + await addAgentToDepartment(api, { department: departmentB, agentId: agent2.data._id }) + await expect((await api.post('/settings/Enable_CSP', { value: false })).status()).toBe(200); await expect((await api.post('/settings/Livechat_offline_email', { value: 'test@testing.com' })).status()).toBe(200); }); @@ -253,6 +251,11 @@ test.describe('OC - Livechat API', () => { await poAuxContext.poHomeOmnichannel.sidenav.switchStatus('online'); } + if (testInfo.title === 'OC - Livechat API - setDepartment') { + const { page: pageCtx2 } = await createAuxContext(browser, Users.user2); + poAuxContext2 = { page: pageCtx2, poHomeOmnichannel: new HomeOmnichannel(pageCtx) }; + } + await page.goto('/packages/rocketchat_livechat/assets/demo.html'); }); @@ -264,9 +267,10 @@ test.describe('OC - Livechat API', () => { test.afterAll(async ({ api }) => { await expect((await api.post('/settings/Enable_CSP', { value: true })).status()).toBe(200); await agent.delete(); + await agent2.delete(); + await expect((await api.post('/settings/Omnichannel_enable_department_removal', { value: true })).status()).toBe(200); - const response = await api.delete(`/livechat/department/${depId}`, { name: 'TestDep', email: 'TestDep@email.com' }); - expect(response.status()).toBe(200); + await Promise.all([...departments.map((department) => department.delete())]); await expect((await api.post('/settings/Omnichannel_enable_department_removal', { value: false })).status()).toBe(200); }); @@ -315,6 +319,47 @@ test.describe('OC - Livechat API', () => { }); }); + test('OC - Livechat API - setDepartment', async () => { + const [departmentA, departmentB] = departments.map(({ data }) => data); + const registerGuestVisitor = { + name: faker.person.firstName(), + email: faker.internet.email(), + token: faker.string.uuid(), + department: departmentA._id, + }; + + // Start Chat + await poLiveChat.page.evaluate(() => window.RocketChat.livechat.maximizeWidget()); + await expect(page.frameLocator('#rocketchat-iframe').getByText('Start Chat')).toBeVisible(); + + await poLiveChat.page.evaluate( + (registerGuestVisitor) => window.RocketChat.livechat.registerGuest(registerGuestVisitor), + registerGuestVisitor, + ); + + await expect(page.frameLocator('#rocketchat-iframe').getByText('Start Chat')).not.toBeVisible(); + + await poLiveChat.onlineAgentMessage.type('this_a_test_message_from_visitor'); + await poLiveChat.btnSendMessageToOnlineAgent.click(); + + await test.step('Expect registered guest to be in dep1', async () => { + await poAuxContext.poHomeOmnichannel.sidenav.openChat(registerGuestVisitor.name); + }); + + const depId = departmentB._id; + + await test.step('Expect setDepartment to change a guest department', async () => { + await poLiveChat.page.evaluate( + (depId) => window.RocketChat.livechat.setDepartment(depId), + depId, + ); + }); + + await test.step('Expect registered guest to be in dep2', async () => { + await poAuxContext2.poHomeOmnichannel.sidenav.openChat(registerGuestVisitor.name); + }); + }); + test('OC - Livechat API - registerGuest', async ({ browser }) => { const registerGuestVisitor = { name: faker.person.firstName(), @@ -495,14 +540,14 @@ test.describe('OC - Livechat API', () => { const { page: pageCtx } = await createAuxContext(browser, Users.user1); poAuxContext = { page: pageCtx, poHomeOmnichannel: new HomeOmnichannel(pageCtx) }; - + // This is needed since the livechat will not react to online/offline status changes if already loaded in a page if (testInfo.title === 'Expect onOfflineFormSubmit to trigger callback') { await poAuxContext.poHomeOmnichannel.sidenav.switchStatus('offline'); } else { await poAuxContext.poHomeOmnichannel.sidenav.switchStatus('online'); } - + await page.goto('/packages/rocketchat_livechat/assets/demo.html'); }); @@ -635,7 +680,7 @@ test.describe('OC - Livechat API', () => { await poLiveChat.sendMessage(newVisitor, false); await poLiveChat.onlineAgentMessage.type('this_a_test_message_from_visitor'); await poLiveChat.btnSendMessageToOnlineAgent.click(); - + const watchForTrigger = page.waitForFunction(() => window.onAgentStatusChange === true); @@ -717,5 +762,5 @@ test.describe('OC - Livechat API', () => { }); }); }); - + }); diff --git a/packages/livechat/src/lib/hooks.js b/packages/livechat/src/lib/hooks.js index 311cf0b2a00bc..67e3313719485 100644 --- a/packages/livechat/src/lib/hooks.js +++ b/packages/livechat/src/lib/hooks.js @@ -88,6 +88,7 @@ const api = { const department = departments.find((dep) => dep._id === value || dep.name === value)?._id || ''; updateIframeGuestData({ department }); + store.setState({ department }); if (defaultAgent && defaultAgent.department !== department) { store.setState({ defaultAgent: null }); diff --git a/packages/livechat/src/lib/room.js b/packages/livechat/src/lib/room.js index 1e4f975f18861..c1d2f0163a566 100644 --- a/packages/livechat/src/lib/room.js +++ b/packages/livechat/src/lib/room.js @@ -25,13 +25,13 @@ export const closeChat = async ({ transcriptRequested } = {}) => { await handleTranscript(); } - const { config: { settings: { clearLocalStorageWhenChatEnded } = {} } = {} } = store.state; + const { department, config: { settings: { clearLocalStorageWhenChatEnded } = {} } = {} } = store.state; if (clearLocalStorageWhenChatEnded) { // exclude UI-affecting flags const { iframe: currentIframe } = store.state; const { minimized, visible, undocked, expanded, businessUnit, config, iframe, ...initial } = initialState(); - initial.iframe = { ...currentIframe, guest: {} }; + initial.iframe = { ...currentIframe, guest: { department } }; await store.setState(initial); }