Skip to content

Commit

Permalink
fix: Remove the association between BH and disabled/archived departme…
Browse files Browse the repository at this point in the history
…nts (RocketChat#29543)

Co-authored-by: Murtaza Patrawala <[email protected]>
Co-authored-by: Guilherme Gazzo <[email protected]>
  • Loading branch information
3 people authored Jul 5, 2023
1 parent 4da8801 commit b837cb9
Show file tree
Hide file tree
Showing 27 changed files with 908 additions and 97 deletions.
6 changes: 6 additions & 0 deletions .changeset/funny-coins-trade.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
"@rocket.chat/meteor": patch
"@rocket.chat/model-typings": patch
---

Fixed a problem where disabled department agent's where still being activated when applicable business hours met.
6 changes: 6 additions & 0 deletions .changeset/rotten-spoons-teach.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
"@rocket.chat/meteor": patch
"@rocket.chat/model-typings": patch
---

Fixed logic around Default Business Hours where agents from disabled/archived departments where being omitted from processing at closing time
6 changes: 2 additions & 4 deletions apps/meteor/app/livechat/imports/server/rest/departments.ts
Original file line number Diff line number Diff line change
Expand Up @@ -192,11 +192,9 @@ API.v1.addRoute(
},
{
async post() {
if (await Livechat.archiveDepartment(this.urlParams._id)) {
return API.v1.success();
}
await Livechat.archiveDepartment(this.urlParams._id);

return API.v1.failure();
return API.v1.success();
},
},
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,9 @@ export interface IBusinessHourBehavior {
onDisableBusinessHours(): Promise<void>;
onAddAgentToDepartment(options?: { departmentId: string; agentsId: string[] }): Promise<any>;
onRemoveAgentFromDepartment(options?: Record<string, any>): Promise<any>;
onRemoveDepartment(department?: ILivechatDepartment): Promise<any>;
onRemoveDepartment(options: { department: ILivechatDepartment; agentsIds: string[] }): Promise<any>;
onDepartmentDisabled(department?: ILivechatDepartment): Promise<any>;
onDepartmentArchived(department: Pick<ILivechatDepartment, '_id'>): Promise<void>;
onStartBusinessHours(): Promise<void>;
afterSaveBusinessHours(businessHourData: ILivechatBusinessHour): Promise<void>;
allowAgentChangeServiceStatus(agentId: string): Promise<boolean>;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,12 @@ import moment from 'moment';
import { LivechatBusinessHourTypes } from '@rocket.chat/core-typings';
import type { ILivechatBusinessHour } from '@rocket.chat/core-typings';
import type { AgendaCronJobs } from '@rocket.chat/cron';
import { Users } from '@rocket.chat/models';
import { LivechatDepartment, Users } from '@rocket.chat/models';

import type { IBusinessHourBehavior, IBusinessHourType } from './AbstractBusinessHour';
import { settings } from '../../../settings/server';
import { callbacks } from '../../../../lib/callbacks';
import { closeBusinessHour } from '../../../../ee/app/livechat-enterprise/server/business-hour/Helper';
import { businessHourLogger } from '../lib/logger';

export class BusinessHourManager {
Expand All @@ -28,6 +29,7 @@ export class BusinessHourManager {
await this.createCronJobsForWorkHours();
businessHourLogger.debug('Cron jobs created, setting up callbacks');
this.setupCallbacks();
await this.cleanupDisabledDepartmentReferences();
await this.behavior.onStartBusinessHours();
}

Expand All @@ -38,6 +40,40 @@ export class BusinessHourManager {
await this.behavior.onDisableBusinessHours();
}

async restartManager(): Promise<void> {
await this.stopManager();
await this.startManager();
}

async cleanupDisabledDepartmentReferences(): Promise<void> {
// Get business hours with departments enabled and disabled
const bhWithDepartments = await LivechatDepartment.getBusinessHoursWithDepartmentStatuses();

if (!bhWithDepartments.length) {
// If there are no bh, skip
return;
}

for await (const { _id: businessHourId, validDepartments, invalidDepartments } of bhWithDepartments) {
if (!invalidDepartments.length) {
continue;
}

// If there are no enabled departments, close the business hour
const allDepsAreDisabled = validDepartments.length === 0 && invalidDepartments.length > 0;
if (allDepsAreDisabled) {
const businessHour = await this.getBusinessHour(businessHourId, LivechatBusinessHourTypes.CUSTOM);
if (!businessHour) {
continue;
}
await closeBusinessHour(businessHour);
}

// Remove business hour from disabled departments
await LivechatDepartment.removeBusinessHourFromDepartmentsByIdsAndBusinessHourId(invalidDepartments, businessHourId);
}
}

async allowAgentChangeServiceStatus(agentId: string): Promise<boolean> {
if (!settings.get('Livechat_enable_business_hours')) {
return true;
Expand Down Expand Up @@ -88,6 +124,14 @@ export class BusinessHourManager {
return Users.setLivechatStatusActiveBasedOnBusinessHours(agentId);
}

async restartCronJobsIfNecessary(): Promise<void> {
if (!settings.get('Livechat_enable_business_hours')) {
return;
}

await this.createCronJobsForWorkHours();
}

private setupCallbacks(): void {
callbacks.add(
'livechat.removeAgentDepartment',
Expand All @@ -107,6 +151,18 @@ export class BusinessHourManager {
callbacks.priority.HIGH,
'business-hour-livechat-on-save-agent-department',
);
callbacks.add(
'livechat.afterDepartmentDisabled',
this.behavior.onDepartmentDisabled.bind(this),
callbacks.priority.HIGH,
'business-hour-livechat-on-department-disabled',
);
callbacks.add(
'livechat.afterDepartmentArchived',
this.behavior.onDepartmentArchived.bind(this),
callbacks.priority.HIGH,
'business-hour-livechat-on-department-archived',
);
callbacks.add(
'livechat.onNewAgentCreated',
this.behavior.onNewAgentCreated.bind(this),
Expand All @@ -119,6 +175,8 @@ export class BusinessHourManager {
callbacks.remove('livechat.removeAgentDepartment', 'business-hour-livechat-on-remove-agent-department');
callbacks.remove('livechat.afterRemoveDepartment', 'business-hour-livechat-after-remove-department');
callbacks.remove('livechat.saveAgentDepartment', 'business-hour-livechat-on-save-agent-department');
callbacks.remove('livechat.afterDepartmentDisabled', 'business-hour-livechat-on-department-disabled');
callbacks.remove('livechat.afterDepartmentArchived', 'business-hour-livechat-on-department-archived');
callbacks.remove('livechat.onNewAgentCreated', 'business-hour-livechat-on-agent-created');
}

Expand Down
8 changes: 8 additions & 0 deletions apps/meteor/app/livechat/server/business-hour/Single.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,4 +49,12 @@ export class SingleBusinessHourBehavior extends AbstractBusinessHourBehavior imp
onRemoveDepartment(): Promise<void> {
return Promise.resolve();
}

onDepartmentDisabled(): Promise<void> {
return Promise.resolve();
}

onDepartmentArchived(): Promise<void> {
return Promise.resolve();
}
}
8 changes: 7 additions & 1 deletion apps/meteor/app/livechat/server/lib/Livechat.js
Original file line number Diff line number Diff line change
Expand Up @@ -740,6 +740,8 @@ export const Livechat = {
});
}

// TODO: these kind of actions should be on events instead of here
await LivechatDepartmentAgents.enableAgentsByDepartmentId(_id);
return LivechatDepartmentRaw.unarchiveDepartment(_id);
},

Expand All @@ -754,7 +756,11 @@ export const Livechat = {
});
}

return LivechatDepartmentRaw.archiveDepartment(_id);
await LivechatDepartmentAgents.disableAgentsByDepartmentId(_id);
await LivechatDepartmentRaw.archiveDepartment(_id);

this.logger.debug({ msg: 'Running livechat.afterDepartmentArchived callback for department:', departmentId: _id });
await callbacks.run('livechat.afterDepartmentArchived', department);
},

showConnecting() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,14 @@ type AutoCompleteDepartmentMultipleProps = {
onChange: (value: PaginatedMultiSelectOption[]) => void;
onlyMyDepartments?: boolean;
showArchived?: boolean;
enabled?: boolean;
};

const AutoCompleteDepartmentMultiple = ({
value,
onlyMyDepartments = false,
showArchived = false,
enabled = false,
onChange = () => undefined,
}: AutoCompleteDepartmentMultipleProps) => {
const t = useTranslation();
Expand All @@ -28,8 +30,8 @@ const AutoCompleteDepartmentMultiple = ({

const { itemsList: departmentsList, loadMoreItems: loadMoreDepartments } = useDepartmentsList(
useMemo(
() => ({ filter: debouncedDepartmentsFilter, onlyMyDepartments, ...(showArchived && { showArchived: true }) }),
[debouncedDepartmentsFilter, onlyMyDepartments, showArchived],
() => ({ filter: debouncedDepartmentsFilter, onlyMyDepartments, ...(showArchived && { showArchived: true }), enabled }),
[debouncedDepartmentsFilter, enabled, onlyMyDepartments, showArchived],
),
);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,12 @@ import { isEnterprise } from '../../../license/server/license';
import { businessHourLogger } from '../../../../../app/livechat/server/lib/logger';

const getAllAgentIdsWithoutDepartment = async (): Promise<string[]> => {
const agentIdsWithDepartment = (
await LivechatDepartmentAgents.find({ departmentEnabled: true }, { projection: { agentId: 1 } }).toArray()
).map((dept) => dept.agentId);
// Fetch departments with agents excluding archived ones (disabled ones still can be tied to business hours)
// Then find the agents that are not in any of those departments

const departmentIds = (await LivechatDepartment.findNotArchived({ projection: { _id: 1 } }).toArray()).map(({ _id }) => _id);

const agentIdsWithDepartment = await LivechatDepartmentAgents.findAllAgentsConnectedToListOfDepartments(departmentIds);

const agentIdsWithoutDepartment = (
await Users.findUsersInRolesWithQuery(
Expand Down Expand Up @@ -62,7 +65,10 @@ const getAgentIdsToHandle = async (businessHour: Pick<ILivechatBusinessHour, '_i
).map((dept) => dept.agentId);
};

export const openBusinessHour = async (businessHour: Pick<ILivechatBusinessHour, '_id' | 'type'>): Promise<void> => {
export const openBusinessHour = async (
businessHour: Pick<ILivechatBusinessHour, '_id' | 'type'>,
updateLivechatStatus = true,
): Promise<void> => {
const agentIds = await getAgentIdsToHandle(businessHour);
businessHourLogger.debug({
msg: 'Opening business hour',
Expand All @@ -72,7 +78,9 @@ export const openBusinessHour = async (businessHour: Pick<ILivechatBusinessHour,
});

await Users.addBusinessHourByAgentIds(agentIds, businessHour._id);
await Users.updateLivechatStatusBasedOnBusinessHours();
if (updateLivechatStatus) {
await Users.updateLivechatStatusBasedOnBusinessHours();
}
};

export const closeBusinessHour = async (businessHour: Pick<ILivechatBusinessHour, '_id' | 'type'>): Promise<void> => {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,12 +1,14 @@
import moment from 'moment';
import type { ILivechatDepartment, ILivechatBusinessHour } from '@rocket.chat/core-typings';
import { LivechatDepartment, LivechatDepartmentAgents } from '@rocket.chat/models';
import { LivechatDepartment, LivechatDepartmentAgents, Users } from '@rocket.chat/models';

import type { IBusinessHourBehavior } from '../../../../../app/livechat/server/business-hour/AbstractBusinessHour';
import { AbstractBusinessHourBehavior } from '../../../../../app/livechat/server/business-hour/AbstractBusinessHour';
import { filterBusinessHoursThatMustBeOpened } from '../../../../../app/livechat/server/business-hour/Helper';
import { closeBusinessHour, openBusinessHour, removeBusinessHourByAgentIds } from './Helper';
import { businessHourLogger } from '../../../../../app/livechat/server/lib/logger';
import { bhLogger } from '../lib/logger';
import { settings } from '../../../../../app/settings/server';
import { businessHourManager } from '../../../../../app/livechat/server/business-hour';

interface IBusinessHoursExtraProperties extends ILivechatBusinessHour {
timezoneName: string;
Expand All @@ -19,6 +21,8 @@ export class MultipleBusinessHoursBehavior extends AbstractBusinessHourBehavior
this.onAddAgentToDepartment = this.onAddAgentToDepartment.bind(this);
this.onRemoveAgentFromDepartment = this.onRemoveAgentFromDepartment.bind(this);
this.onRemoveDepartment = this.onRemoveDepartment.bind(this);
this.onDepartmentArchived = this.onDepartmentArchived.bind(this);
this.onDepartmentDisabled = this.onDepartmentDisabled.bind(this);
this.onNewAgentCreated = this.onNewAgentCreated.bind(this);
}

Expand All @@ -36,7 +40,7 @@ export class MultipleBusinessHoursBehavior extends AbstractBusinessHourBehavior
},
});
const businessHoursToOpen = await filterBusinessHoursThatMustBeOpened(activeBusinessHours);
businessHourLogger.debug({
bhLogger.debug({
msg: 'Starting Multiple Business Hours',
totalBusinessHoursToOpen: businessHoursToOpen.length,
top10BusinessHoursToOpen: businessHoursToOpen.slice(0, 10),
Expand Down Expand Up @@ -125,13 +129,100 @@ export class MultipleBusinessHoursBehavior extends AbstractBusinessHourBehavior
return this.handleRemoveAgentsFromDepartments(department, agentsId, options);
}

async onRemoveDepartment(options: Record<string, any> = {}): Promise<any> {
async onRemoveDepartment(options: { department: ILivechatDepartment; agentsIds: string[] }): Promise<any> {
bhLogger.debug(`onRemoveDepartment: department ${options.department._id} removed`);
const { department, agentsIds } = options;
if (!department || !agentsIds?.length) {
return options;
}
const deletedDepartment = LivechatDepartment.trashFindOneById(department._id);
return this.handleRemoveAgentsFromDepartments(deletedDepartment, agentsIds, options);
return this.onDepartmentDisabled(department);
}

async onDepartmentDisabled(department: ILivechatDepartment): Promise<void> {
if (!department.businessHourId) {
bhLogger.debug({
msg: 'onDepartmentDisabled: department has no business hour',
departmentId: department._id,
});
return;
}

// Get business hour
let businessHour = await this.BusinessHourRepository.findOneById(department.businessHourId);
if (!businessHour) {
bhLogger.error({
msg: 'onDepartmentDisabled: business hour not found',
businessHourId: department.businessHourId,
});
return;
}

// Unlink business hour from department
await LivechatDepartment.removeBusinessHourFromDepartmentsByIdsAndBusinessHourId([department._id], businessHour._id);

// cleanup user's cache for default business hour and this business hour
const defaultBH = await this.BusinessHourRepository.findOneDefaultBusinessHour();
if (!defaultBH) {
bhLogger.error('onDepartmentDisabled: default business hour not found');
throw new Error('Default business hour not found');
}
await this.UsersRepository.closeAgentsBusinessHoursByBusinessHourIds([businessHour._id, defaultBH._id]);

// If i'm the only one, disable the business hour
const imTheOnlyOne = !(await LivechatDepartment.countByBusinessHourIdExcludingDepartmentId(businessHour._id, department._id));
if (imTheOnlyOne) {
bhLogger.warn({
msg: 'onDepartmentDisabled: department is the only one on business hour, disabling it',
departmentId: department._id,
businessHourId: businessHour._id,
});
await this.BusinessHourRepository.disableBusinessHour(businessHour._id);

businessHour = await this.BusinessHourRepository.findOneById(department.businessHourId);
if (!businessHour) {
bhLogger.error({
msg: 'onDepartmentDisabled: business hour not found',
businessHourId: department.businessHourId,
});

throw new Error(`Business hour ${department.businessHourId} not found`);
}
}

// start default business hour and this BH if needed
if (!settings.get('Livechat_enable_business_hours')) {
bhLogger.debug(`onDepartmentDisabled: business hours are disabled. skipping`);
return;
}
const businessHourToOpen = await filterBusinessHoursThatMustBeOpened([businessHour, defaultBH]);
for await (const bh of businessHourToOpen) {
bhLogger.debug({
msg: 'onDepartmentDisabled: opening business hour',
businessHourId: bh._id,
});
await openBusinessHour(bh, false);
}

await Users.updateLivechatStatusBasedOnBusinessHours();

await businessHourManager.restartCronJobsIfNecessary();

bhLogger.debug({
msg: 'onDepartmentDisabled: successfully processed department disabled event',
departmentId: department._id,
});
}

async onDepartmentArchived(department: Pick<ILivechatDepartment, '_id'>): Promise<void> {
bhLogger.debug('Processing department archived event on multiple business hours', department);
const dbDepartment = await LivechatDepartment.findOneById(department._id, { projection: { businessHourId: 1, _id: 1 } });

if (!dbDepartment) {
bhLogger.error(`No department found with id: ${department._id} when archiving it`);
return;
}

return this.onDepartmentDisabled(dbDepartment);
}

allowAgentChangeServiceStatus(agentId: string): Promise<boolean> {
Expand All @@ -147,7 +238,6 @@ export class MultipleBusinessHoursBehavior extends AbstractBusinessHourBehavior
}
// TODO: We're doing a full fledged aggregation with lookups and getting the whole array just for getting the length? :(
if (!(await LivechatDepartmentAgents.findAgentsByAgentIdAndBusinessHourId(agentId, department.businessHourId)).length) {
// eslint-disable-line no-await-in-loop
agentIdsToRemoveCurrentBusinessHour.push(agentId);
}
}
Expand Down
Loading

0 comments on commit b837cb9

Please sign in to comment.