-
Notifications
You must be signed in to change notification settings - Fork 10.9k
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
fix:
QueueInactivityMonitor
not processing inquiries (#32452)
- Loading branch information
Showing
3 changed files
with
135 additions
and
10 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
--- | ||
"@rocket.chat/meteor": patch | ||
--- | ||
|
||
Fixed 2 issues with `QueueInactivityMonitor` callback. This callback was in charge of scheduling the job that would close the inquiry, but it was checking on a property that didn't exist. This caused the callback to early return without scheduling the job, making the feature to not to work. |
20 changes: 10 additions & 10 deletions
20
apps/meteor/ee/app/livechat-enterprise/server/hooks/afterInquiryQueued.ts
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,32 +1,32 @@ | ||
import type { ILivechatInquiryRecord } from '@rocket.chat/core-typings'; | ||
import moment from 'moment'; | ||
|
||
import { settings } from '../../../../../app/settings/server'; | ||
import { callbacks } from '../../../../../lib/callbacks'; | ||
import { OmnichannelQueueInactivityMonitor } from '../lib/QueueInactivityMonitor'; | ||
import { cbLogger } from '../lib/logger'; | ||
|
||
let timer = 0; | ||
|
||
const scheduleInquiry = async (inquiry: any): Promise<void> => { | ||
if (!inquiry?._id) { | ||
export const afterInquiryQueued = async (inquiry: ILivechatInquiryRecord) => { | ||
if (!inquiry?._id || !inquiry?._updatedAt) { | ||
return; | ||
} | ||
|
||
if (!inquiry?._updatedAt || !inquiry?._createdAt) { | ||
const timer = settings.get<number>('Livechat_max_queue_wait_time'); | ||
|
||
if (timer <= 0) { | ||
return; | ||
} | ||
|
||
// schedule individual jobs instead of property for close inactivty | ||
const newQueueTime = moment(inquiry._updatedAt || inquiry._createdAt).add(timer, 'minutes'); | ||
const newQueueTime = moment(inquiry._updatedAt).add(timer, 'minutes'); | ||
cbLogger.debug(`Scheduling estimated close time at ${newQueueTime} for queued inquiry ${inquiry._id}`); | ||
await OmnichannelQueueInactivityMonitor.scheduleInquiry(inquiry._id, new Date(newQueueTime.format())); | ||
}; | ||
|
||
settings.watch('Livechat_max_queue_wait_time', (value) => { | ||
timer = value as number; | ||
if (timer <= 0) { | ||
settings.watch<number>('Livechat_max_queue_wait_time', (value) => { | ||
if (value <= 0) { | ||
callbacks.remove('livechat.afterInquiryQueued', 'livechat-inquiry-queued-set-queue-timer'); | ||
return; | ||
} | ||
callbacks.add('livechat.afterInquiryQueued', scheduleInquiry, callbacks.priority.HIGH, 'livechat-inquiry-queued-set-queue-timer'); | ||
callbacks.add('livechat.afterInquiryQueued', afterInquiryQueued, callbacks.priority.HIGH, 'livechat-inquiry-queued-set-queue-timer'); | ||
}); |
120 changes: 120 additions & 0 deletions
120
apps/meteor/ee/tests/unit/apps/livechat-enterprise/server/hooks/afterInquiryQueued.spec.ts
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,120 @@ | ||
import { expect } from 'chai'; | ||
import { describe, it } from 'mocha'; | ||
import moment from 'moment'; | ||
import proxyquire from 'proxyquire'; | ||
import sinon from 'sinon'; | ||
|
||
const settingStub = { | ||
watch: sinon.stub(), | ||
get: sinon.stub(), | ||
}; | ||
|
||
const callbackStub = { | ||
add: sinon.stub(), | ||
remove: sinon.stub(), | ||
priority: { HIGH: 'high' }, | ||
}; | ||
|
||
const queueMonitorStub = { | ||
scheduleInquiry: sinon.stub(), | ||
}; | ||
|
||
const { afterInquiryQueued } = proxyquire | ||
.noCallThru() | ||
.load('../../../../../../app/livechat-enterprise/server/hooks/afterInquiryQueued.ts', { | ||
'../../../../../app/settings/server': { | ||
settings: settingStub, | ||
}, | ||
'../../../../../lib/callbacks': { | ||
callbacks: callbackStub, | ||
}, | ||
'../lib/QueueInactivityMonitor': { | ||
OmnichannelQueueInactivityMonitor: queueMonitorStub, | ||
}, | ||
'../lib/logger': { | ||
cbLogger: { debug: sinon.stub() }, | ||
}, | ||
}); | ||
|
||
describe('hooks/afterInquiryQueued', () => { | ||
beforeEach(() => { | ||
callbackStub.add.resetHistory(); | ||
callbackStub.remove.resetHistory(); | ||
queueMonitorStub.scheduleInquiry.resetHistory(); | ||
settingStub.get.resetHistory(); | ||
}); | ||
|
||
it('should call settings.watch at first', () => { | ||
expect(settingStub.watch.callCount).to.be.equal(1); | ||
}); | ||
|
||
it('should call the callback on settings.watch with proper values', () => { | ||
const func = settingStub.watch.getCall(0).args[1]; | ||
|
||
func(1); | ||
expect(callbackStub.add.callCount).to.be.equal(1); | ||
|
||
func(2); | ||
expect(callbackStub.add.callCount).to.be.equal(2); | ||
|
||
func(0); | ||
expect(callbackStub.remove.callCount).to.be.equal(1); | ||
|
||
func(-1); | ||
expect(callbackStub.remove.callCount).to.be.equal(2); | ||
|
||
func(3); | ||
expect(callbackStub.add.callCount).to.be.equal(3); | ||
}); | ||
|
||
it('should return undefined if no inquiry is passed, or if inquiry doesnt have valid properties', async () => { | ||
expect(await afterInquiryQueued(null)).to.be.equal(undefined); | ||
expect(await afterInquiryQueued({})).to.be.equal(undefined); | ||
expect(await afterInquiryQueued({ _id: 'invalid' })).to.be.equal(undefined); | ||
expect(await afterInquiryQueued({ _updatedAt: new Date() })); | ||
expect(await afterInquiryQueued({ _updatedAt: null, _id: 'afsd34asdX' })).to.be.equal(undefined); | ||
}); | ||
|
||
it('should do nothing if timer is set to 0 or less', async () => { | ||
const inquiry = { | ||
_id: 'afsd34asdX', | ||
_updatedAt: new Date(), | ||
}; | ||
|
||
settingStub.get.returns(0); | ||
await afterInquiryQueued(inquiry); | ||
expect(queueMonitorStub.scheduleInquiry.callCount).to.be.equal(0); | ||
|
||
settingStub.get.returns(-1); | ||
await afterInquiryQueued(inquiry); | ||
expect(queueMonitorStub.scheduleInquiry.callCount).to.be.equal(0); | ||
}); | ||
|
||
it('should call .scheduleInquiry with proper data', async () => { | ||
const inquiry = { | ||
_id: 'afsd34asdX', | ||
_updatedAt: new Date(), | ||
}; | ||
|
||
settingStub.get.returns(1); | ||
await afterInquiryQueued(inquiry); | ||
|
||
const newQueueTime = moment(inquiry._updatedAt).add(1, 'minutes'); | ||
|
||
expect(queueMonitorStub.scheduleInquiry.calledWith(inquiry._id, new Date(newQueueTime.format()))).to.be.true; | ||
}); | ||
|
||
it('should call .scheduleInquiry with proper data when more than 1 min is passed as param', async () => { | ||
const inquiry = { | ||
_id: 'afv34avzx', | ||
_updatedAt: new Date(), | ||
}; | ||
|
||
settingStub.get.returns(3); | ||
await afterInquiryQueued(inquiry); | ||
|
||
const newQueueTime = moment(inquiry._updatedAt).add(3, 'minutes'); | ||
|
||
expect(queueMonitorStub.scheduleInquiry.calledWith(inquiry._id, new Date(newQueueTime.format()))).to.be.true; | ||
}); | ||
}); |