Skip to content

Commit

Permalink
Merge branch 'develop' into refactor/set-reactions
Browse files Browse the repository at this point in the history
  • Loading branch information
@KevLehman
KevLehman authored Aug 20, 2024
2 parents f6243d4 + 51f2fc2 commit d2a6873
Show file tree
Hide file tree
Showing 4 changed files with 763 additions and 123 deletions.
6 changes: 6 additions & 0 deletions .changeset/spicy-kings-think.md
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
"@rocket.chat/meteor": patch
---

Fixes multiple problems with the `processRoomAbandonment` hook. This hook is in charge of calculating the time a room has been abandoned (this means, the time that elapsed since a room was last answered by an agent until it was closed). However, when business hours were enabled and the user didn't open on one day, if an abandoned room happened to be abandoned _over_ the day there was no business hour configuration, then the process will error out.
Additionally, the values the code was calculating were not right. When business hours are enabled, this code should only count the abandonment time _while a business hour was open_. When rooms were left abandoned for days or weeks, this will also throw an error or output an invalid count.
111 changes: 66 additions & 45 deletions apps/meteor/app/livechat/server/hooks/processRoomAbandonment.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,12 @@ import moment from 'moment';
import { callbacks } from '../../../../lib/callbacks';
import { settings } from '../../../settings/server';
import { businessHourManager } from '../business-hour';
import type { CloseRoomParams } from '../lib/localTypes';

const getSecondsWhenOfficeHoursIsDisabled = (room: IOmnichannelRoom, agentLastMessage: IMessage) =>
export const getSecondsWhenOfficeHoursIsDisabled = (room: IOmnichannelRoom, agentLastMessage: IMessage) =>
moment(new Date(room.closedAt || new Date())).diff(moment(new Date(agentLastMessage.ts)), 'seconds');

const parseDays = (
export const parseDays = (
acc: Record<string, { start: { day: string; time: string }; finish: { day: string; time: string }; open: boolean }>,
day: IBusinessHourWorkHour,
) => {
Expand All @@ -22,7 +23,7 @@ const parseDays = (
return acc;
};

const getSecondsSinceLastAgentResponse = async (room: IOmnichannelRoom, agentLastMessage: IMessage) => {
export const getSecondsSinceLastAgentResponse = async (room: IOmnichannelRoom, agentLastMessage: IMessage) => {
if (!settings.get('Livechat_enable_business_hours')) {
return getSecondsWhenOfficeHoursIsDisabled(room, agentLastMessage);
}
Expand All @@ -49,65 +50,85 @@ const getSecondsSinceLastAgentResponse = async (room: IOmnichannelRoom, agentLas
}

let totalSeconds = 0;
const endOfConversation = moment(new Date(room.closedAt || new Date()));
const startOfInactivity = moment(new Date(agentLastMessage.ts));
const endOfConversation = moment.utc(new Date(room.closedAt || new Date()));
const startOfInactivity = moment.utc(new Date(agentLastMessage.ts));
const daysOfInactivity = endOfConversation.clone().startOf('day').diff(startOfInactivity.clone().startOf('day'), 'days');
const inactivityDay = moment(new Date(agentLastMessage.ts));
const inactivityDay = moment.utc(new Date(agentLastMessage.ts));

for (let index = 0; index <= daysOfInactivity; index++) {
const today = inactivityDay.clone().format('dddd');
const officeDay = officeDays[today];
// Config doesnt have data for this day, we skip day
if (!officeDay) {
inactivityDay.add(1, 'days');
continue;
}
const startTodaysOfficeHour = moment(`${officeDay.start.day}:${officeDay.start.time}`, 'dddd:HH:mm').add(index, 'days');
const endTodaysOfficeHour = moment(`${officeDay.finish.day}:${officeDay.finish.time}`, 'dddd:HH:mm').add(index, 'days');
if (officeDays[today].open) {
const firstDayOfInactivity = startOfInactivity.clone().format('D') === inactivityDay.clone().format('D');
const lastDayOfInactivity = endOfConversation.clone().format('D') === inactivityDay.clone().format('D');

if (!firstDayOfInactivity && !lastDayOfInactivity) {
totalSeconds += endTodaysOfficeHour.clone().diff(startTodaysOfficeHour, 'seconds');
} else {
const end = endOfConversation.isBefore(endTodaysOfficeHour) ? endOfConversation : endTodaysOfficeHour;
const start = firstDayOfInactivity ? inactivityDay : startTodaysOfficeHour;
totalSeconds += end.clone().diff(start, 'seconds');
}
if (!officeDay.open) {
inactivityDay.add(1, 'days');
continue;
}
if (!officeDay?.start?.time || !officeDay?.finish?.time) {
inactivityDay.add(1, 'days');
continue;
}
inactivityDay.add(1, 'days');
}
return totalSeconds;
};

callbacks.add(
'livechat.closeRoom',
async (params) => {
const { room } = params;
const [officeStartHour, officeStartMinute] = officeDay.start.time.split(':');
const [officeCloseHour, officeCloseMinute] = officeDay.finish.time.split(':');
// We should only take in consideration the time where the office is open and the conversation was inactive
const todayStartOfficeHours = inactivityDay
.clone()
.set({ hour: parseInt(officeStartHour, 10), minute: parseInt(officeStartMinute, 10) });
const todayEndOfficeHours = inactivityDay.clone().set({ hour: parseInt(officeCloseHour, 10), minute: parseInt(officeCloseMinute, 10) });

if (!isOmnichannelRoom(room)) {
return params;
// 1: Room was inactive the whole day, we add the whole time BH is inactive
if (startOfInactivity.isBefore(todayStartOfficeHours) && endOfConversation.isAfter(todayEndOfficeHours)) {
totalSeconds += todayEndOfficeHours.diff(todayStartOfficeHours, 'seconds');
}

const closedByAgent = room.closer !== 'visitor';
const wasTheLastMessageSentByAgent = room.lastMessage && !room.lastMessage.token;
if (!closedByAgent || !wasTheLastMessageSentByAgent) {
return params;
// 2: Room was inactive before start but was closed before end of BH, we add the inactive time
if (startOfInactivity.isBefore(todayStartOfficeHours) && endOfConversation.isBefore(todayEndOfficeHours)) {
totalSeconds += endOfConversation.diff(todayStartOfficeHours, 'seconds');
}

if (!room.v?.lastMessageTs) {
return params;
// 3: Room was inactive after start and ended after end of BH, we add the inactive time
if (startOfInactivity.isAfter(todayStartOfficeHours) && endOfConversation.isAfter(todayEndOfficeHours)) {
totalSeconds += todayEndOfficeHours.diff(startOfInactivity, 'seconds');
}

const agentLastMessage = await Messages.findAgentLastMessageByVisitorLastMessageTs(room._id, room.v.lastMessageTs);
if (!agentLastMessage) {
return params;
// 4: Room was inactive after start and before end of BH, we add the inactive time
if (startOfInactivity.isAfter(todayStartOfficeHours) && endOfConversation.isBefore(todayEndOfficeHours)) {
totalSeconds += endOfConversation.diff(startOfInactivity, 'seconds');
}
const secondsSinceLastAgentResponse = await getSecondsSinceLastAgentResponse(room, agentLastMessage);
await LivechatRooms.setVisitorInactivityInSecondsById(room._id, secondsSinceLastAgentResponse);

inactivityDay.add(1, 'days');
}
return totalSeconds;
};

export const onCloseRoom = async (params: { room: IOmnichannelRoom; options: CloseRoomParams['options'] }) => {
const { room } = params;

if (!isOmnichannelRoom(room)) {
return params;
}

const closedByAgent = room.closer !== 'visitor';
const wasTheLastMessageSentByAgent = room.lastMessage && !room.lastMessage.token;
if (!closedByAgent || !wasTheLastMessageSentByAgent) {
return params;
}

if (!room.v?.lastMessageTs) {
return params;
},
callbacks.priority.HIGH,
'process-room-abandonment',
);
}

const agentLastMessage = await Messages.findAgentLastMessageByVisitorLastMessageTs(room._id, room.v.lastMessageTs);
if (!agentLastMessage) {
return params;
}
const secondsSinceLastAgentResponse = await getSecondsSinceLastAgentResponse(room, agentLastMessage);
await LivechatRooms.setVisitorInactivityInSecondsById(room._id, secondsSinceLastAgentResponse);

return params;
};

callbacks.add('livechat.closeRoom', onCloseRoom, callbacks.priority.HIGH, 'process-room-abandonment');
146 changes: 68 additions & 78 deletions apps/meteor/tests/e2e/omnichannel/omnichannel-departaments.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,101 +22,102 @@ test.describe('OC - Manage Departments', () => {

test.beforeAll(async ({ api }) => {
// turn on department removal
const statusCode = (await api.post('/settings/Omnichannel_enable_department_removal', { value: true })).status();
await expect(statusCode).toBe(200);
await api.post('/settings/Omnichannel_enable_department_removal', { value: true });
});

test.afterAll(async ({ api }) => {
// turn off department removal
const statusCode = (await api.post('/settings/Omnichannel_enable_department_removal', { value: false })).status();
await expect(statusCode).toBe(200);
await api.post('/settings/Omnichannel_enable_department_removal', { value: false });
});

test.beforeEach(async ({ page }: { page: Page }) => {
poOmnichannelDepartments = new OmnichannelDepartments(page);
test.describe('Create first department', async () => {
test.beforeEach(async ({ page }: { page: Page }) => {
poOmnichannelDepartments = new OmnichannelDepartments(page);

await page.goto('/omnichannel');
await poOmnichannelDepartments.sidenav.linkDepartments.click();
});

test('Create department', async () => {
const departmentName = faker.string.uuid();
await page.goto('/omnichannel');
await poOmnichannelDepartments.sidenav.linkDepartments.click();
});

await poOmnichannelDepartments.headingButtonNew('Create department').click();
test('Create department', async () => {
const departmentName = faker.string.uuid();

await test.step('expect name and email to be required', async () => {
await expect(poOmnichannelDepartments.invalidInputEmail).not.toBeVisible();
await poOmnichannelDepartments.inputName.fill('any_text');
await poOmnichannelDepartments.inputName.fill('');
await expect(poOmnichannelDepartments.invalidInputName).toBeVisible();
await expect(poOmnichannelDepartments.errorMessage(ERROR.requiredName)).toBeVisible();
await poOmnichannelDepartments.inputName.fill('any_text');
await expect(poOmnichannelDepartments.invalidInputName).not.toBeVisible();
await poOmnichannelDepartments.headingButtonNew('Create department').click();

await poOmnichannelDepartments.inputEmail.fill('any_text');
await expect(poOmnichannelDepartments.invalidInputEmail).toBeVisible();
await expect(poOmnichannelDepartments.errorMessage(ERROR.invalidEmail)).toBeVisible();
await test.step('expect name and email to be required', async () => {
await expect(poOmnichannelDepartments.invalidInputEmail).not.toBeVisible();
await poOmnichannelDepartments.inputName.fill('any_text');
await poOmnichannelDepartments.inputName.fill('');
await expect(poOmnichannelDepartments.invalidInputName).toBeVisible();
await expect(poOmnichannelDepartments.errorMessage(ERROR.requiredName)).toBeVisible();
await poOmnichannelDepartments.inputName.fill('any_text');
await expect(poOmnichannelDepartments.invalidInputName).not.toBeVisible();

await poOmnichannelDepartments.inputEmail.fill('');
await expect(poOmnichannelDepartments.invalidInputEmail).toBeVisible();
await expect(poOmnichannelDepartments.errorMessage(ERROR.requiredEmail)).toBeVisible();
await poOmnichannelDepartments.inputEmail.fill('any_text');
await expect(poOmnichannelDepartments.invalidInputEmail).toBeVisible();
await expect(poOmnichannelDepartments.errorMessage(ERROR.invalidEmail)).toBeVisible();

await poOmnichannelDepartments.inputEmail.fill(faker.internet.email());
await expect(poOmnichannelDepartments.invalidInputEmail).not.toBeVisible();
await expect(poOmnichannelDepartments.errorMessage(ERROR.requiredEmail)).not.toBeVisible();
});
await poOmnichannelDepartments.inputEmail.fill('');
await expect(poOmnichannelDepartments.invalidInputEmail).toBeVisible();
await expect(poOmnichannelDepartments.errorMessage(ERROR.requiredEmail)).toBeVisible();

await test.step('expect create new department', async () => {
await poOmnichannelDepartments.btnEnabled.click();
await poOmnichannelDepartments.inputName.fill(departmentName);
await poOmnichannelDepartments.inputEmail.fill(faker.internet.email());
await poOmnichannelDepartments.btnSave.click();
await poOmnichannelDepartments.btnCloseToastSuccess.click();

await poOmnichannelDepartments.search(departmentName);
await expect(poOmnichannelDepartments.firstRowInTable).toBeVisible();
});
await poOmnichannelDepartments.inputEmail.fill(faker.internet.email());
await expect(poOmnichannelDepartments.invalidInputEmail).not.toBeVisible();
await expect(poOmnichannelDepartments.errorMessage(ERROR.requiredEmail)).not.toBeVisible();
});

await test.step('expect to delete department', async () => {
await poOmnichannelDepartments.search(departmentName);
await poOmnichannelDepartments.selectedDepartmentMenu(departmentName).click();
await poOmnichannelDepartments.menuDeleteOption.click();
await test.step('expect create new department', async () => {
await poOmnichannelDepartments.btnEnabled.click();
await poOmnichannelDepartments.inputName.fill(departmentName);
await poOmnichannelDepartments.inputEmail.fill(faker.internet.email());
await poOmnichannelDepartments.btnSave.click();

await test.step('expect confirm delete department', async () => {
await expect(poOmnichannelDepartments.modalConfirmDelete).toBeVisible();
await poOmnichannelDepartments.search(departmentName);
await expect(poOmnichannelDepartments.firstRowInTable).toBeVisible();
});

await test.step('expect delete to be disabled when name is incorrect', async () => {
await expect(poOmnichannelDepartments.btnModalConfirmDelete).toBeDisabled();
await poOmnichannelDepartments.inputModalConfirmDelete.fill('someramdomname');
await expect(poOmnichannelDepartments.btnModalConfirmDelete).toBeDisabled();
await test.step('expect to delete department', async () => {
await poOmnichannelDepartments.search(departmentName);
await poOmnichannelDepartments.selectedDepartmentMenu(departmentName).click();
await poOmnichannelDepartments.menuDeleteOption.click();

await test.step('expect confirm delete department', async () => {
await test.step('expect delete to be disabled when name is incorrect', async () => {
await expect(poOmnichannelDepartments.btnModalConfirmDelete).toBeDisabled();
await poOmnichannelDepartments.inputModalConfirmDelete.fill('someramdomname');
await expect(poOmnichannelDepartments.btnModalConfirmDelete).toBeDisabled();
});

await test.step('expect to successfuly delete if department name is correct', async () => {
await expect(poOmnichannelDepartments.btnModalConfirmDelete).toBeDisabled();
await poOmnichannelDepartments.inputModalConfirmDelete.fill(departmentName);
await expect(poOmnichannelDepartments.btnModalConfirmDelete).toBeEnabled();
await poOmnichannelDepartments.btnModalConfirmDelete.click();
});
});

await test.step('expect to successfuly delete if department name is correct', async () => {
await expect(poOmnichannelDepartments.btnModalConfirmDelete).toBeDisabled();
await poOmnichannelDepartments.inputModalConfirmDelete.fill(departmentName);
await expect(poOmnichannelDepartments.btnModalConfirmDelete).toBeEnabled();
await poOmnichannelDepartments.btnModalConfirmDelete.click();
await test.step('expect department to have been deleted', async () => {
await poOmnichannelDepartments.search(departmentName);
await expect(poOmnichannelDepartments.firstRowInTable).toHaveCount(0);
});
});

await test.step('expect department to have been deleted', async () => {
await poOmnichannelDepartments.search(departmentName);
await expect(poOmnichannelDepartments.firstRowInTable).toHaveCount(0);
});
});
});

test.describe('After creation', async () => {
let department: Awaited<ReturnType<typeof createDepartment>>['data'];
test.beforeEach(async ({ api }) => {

test.beforeEach(async ({ api, page }) => {
poOmnichannelDepartments = new OmnichannelDepartments(page);

department = await createDepartment(api).then((res) => res.data);
await page.goto('/omnichannel/departments');
});

test.afterEach(async ({ api }) => {
await deleteDepartment(api, { id: department._id });
});

test('Edit department', async ({ api }) => {
test('Edit department', async () => {
await test.step('expect create new department', async () => {
await poOmnichannelDepartments.search(department.name);
await expect(poOmnichannelDepartments.firstRowInTable).toBeVisible();
Expand All @@ -132,19 +133,13 @@ test.describe('OC - Manage Departments', () => {

await poOmnichannelDepartments.inputName.fill(`edited-${department.name}`);
await poOmnichannelDepartments.btnSave.click();
await poOmnichannelDepartments.btnCloseToastSuccess.click();

await poOmnichannelDepartments.search(`edited-${department.name}`);
await expect(poOmnichannelDepartments.firstRowInTable).toBeVisible();
});

await test.step('expect to delete department', async () => {
const deleteRes = await deleteDepartment(api, { id: department._id });
await expect(deleteRes.status()).toBe(200);
});
});

test('Archive department', async ({ api }) => {
test('Archive department', async () => {
await test.step('expect create new department', async () => {
await poOmnichannelDepartments.search(department.name);
await expect(poOmnichannelDepartments.firstRowInTable).toBeVisible();
Expand Down Expand Up @@ -172,11 +167,6 @@ test.describe('OC - Manage Departments', () => {
await poOmnichannelDepartments.menuUnarchiveOption.click();
await expect(poOmnichannelDepartments.firstRowInTable).toHaveCount(0);
});

await test.step('expect to delete department', async () => {
const deleteRes = await deleteDepartment(api, { id: department._id });
await expect(deleteRes.status()).toBe(200);
});
});

test('Request tag(s) before closing conversation', async () => {
Expand Down Expand Up @@ -269,7 +259,7 @@ test.describe('OC - Manage Departments', () => {

await test.step('expect to disable department removal setting', async () => {
const statusCode = (await api.post('/settings/Omnichannel_enable_department_removal', { value: false })).status();
await expect(statusCode).toBe(200);
expect(statusCode).toBe(200);
});

await test.step('expect not to be able to delete department', async () => {
Expand All @@ -280,12 +270,12 @@ test.describe('OC - Manage Departments', () => {

await test.step('expect to enable department removal setting', async () => {
const statusCode = (await api.post('/settings/Omnichannel_enable_department_removal', { value: true })).status();
await expect(statusCode).toBe(200);
expect(statusCode).toBe(200);
});

await test.step('expect to delete department', async () => {
const deleteRes = await deleteDepartment(api, { id: department._id });
await expect(deleteRes.status()).toBe(200);
expect(deleteRes.status()).toBe(200);
});
});
});
Expand Down
Loading

0 comments on commit d2a6873

Please sign in to comment.