Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Blank selected department when selecting out of range #33386

Merged
merged 13 commits into from
Oct 9, 2024
7 changes: 7 additions & 0 deletions .changeset/sharp-adults-think.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
"@rocket.chat/meteor": minor
---

Fixes the departments filter on the omnichannel current chats page by ensuring that the selected department is fetched and
added if it was not part of the initial department list. This prevents the filter from becoming blank and avoids potential
UX issues.
3 changes: 2 additions & 1 deletion apps/meteor/client/components/AutoCompleteDepartment.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,9 @@ const AutoCompleteDepartment = ({
haveNone,
excludeDepartmentId,
showArchived,
selectedDepartment: value,
}),
[debouncedDepartmentsFilter, onlyMyDepartments, haveAll, haveNone, excludeDepartmentId, showArchived],
[debouncedDepartmentsFilter, onlyMyDepartments, haveAll, haveNone, excludeDepartmentId, showArchived, value],
),
);

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
export type DepartmentListItem = {
_id: string;
label: string;
value: string;
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,111 @@
import { mockAppRoot } from '@rocket.chat/mock-providers';
import { renderHook, waitFor } from '@testing-library/react';

import { useDepartmentsList } from './useDepartmentsList';

const initialDepartmentsListMock = Array.from(Array(25)).map((_, index) => {
return {
_id: `${index}`,
name: `test_department_${index}`,
enabled: true,
email: `test${index}@email.com`,
showOnRegistration: false,
showOnOfflineForm: false,
type: 'd',
_updatedAt: '2024-09-26T20:05:31.330Z',
offlineMessageChannelName: '',
numAgents: 0,
ancestors: undefined,
parentId: undefined,
};
});

it('should not fetch and add selected department if it is already in the departments list on first fetch', async () => {
const selectedDepartmentMappedToOption = {
_id: '5',
label: 'test_department_5',
value: '5',
};

const getDepartmentByIdCallback = jest.fn();

const { result } = renderHook(
() =>
useDepartmentsList({
filter: '',
onlyMyDepartments: true,
haveAll: true,
showArchived: true,
selectedDepartment: '5',
}),
{
legacyRoot: true,
wrapper: mockAppRoot()
.withEndpoint('GET', '/v1/livechat/department', () => ({
count: 25,
offset: 0,
total: 25,
departments: initialDepartmentsListMock,
}))
.withEndpoint('GET', `/v1/livechat/department/:_id`, getDepartmentByIdCallback)
.build(),
},
);

expect(getDepartmentByIdCallback).not.toHaveBeenCalled();
await waitFor(() => expect(result.current.itemsList.items).toContainEqual(selectedDepartmentMappedToOption));
// The expected length is 26 because the hook will add the 'All' item on run time
await waitFor(() => expect(result.current.itemsList.items.length).toBe(26));
});

it('should fetch and add selected department if it is not part of departments list on first fetch', async () => {
const missingDepartmentRawMock = {
_id: '56f5be8bcf8cd67f9e9bcfdc',
name: 'test_department_25',
enabled: true,
email: 'test25@email.com',
showOnRegistration: false,
showOnOfflineForm: false,
type: 'd',
_updatedAt: '2024-09-26T20:05:31.330Z',
offlineMessageChannelName: '',
numAgents: 0,
ancestors: undefined,
parentId: undefined,
};

const missingDepartmentMappedToOption = {
_id: '56f5be8bcf8cd67f9e9bcfdc',
label: 'test_department_25',
value: '56f5be8bcf8cd67f9e9bcfdc',
};

const { result } = renderHook(
() =>
useDepartmentsList({
filter: '',
onlyMyDepartments: true,
haveAll: true,
showArchived: true,
selectedDepartment: '56f5be8bcf8cd67f9e9bcfdc',
}),
{
legacyRoot: true,
wrapper: mockAppRoot()
.withEndpoint('GET', '/v1/livechat/department', () => ({
count: 25,
offset: 0,
total: 25,
departments: initialDepartmentsListMock,
}))
.withEndpoint('GET', `/v1/livechat/department/:_id`, () => ({
department: missingDepartmentRawMock,
}))
.build(),
},
);

await waitFor(() => expect(result.current.itemsList.items).toContainEqual(missingDepartmentMappedToOption));
// The expected length is 27 because the hook will add the 'All' item and the missing department on run time
await waitFor(() => expect(result.current.itemsList.items.length).toBe(27));
});
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ import { useCallback, useState } from 'react';
import { useScrollableRecordList } from '../../../hooks/lists/useScrollableRecordList';
import { useComponentDidUpdate } from '../../../hooks/useComponentDidUpdate';
import { RecordList } from '../../../lib/lists/RecordList';
import type { DepartmentListItem } from '../Definitions/DepartmentsDefinitions';
import { normalizeDepartments } from '../utils/normalizeDepartments';

type DepartmentsListOptions = {
filter: string;
Expand All @@ -14,12 +16,7 @@ type DepartmentsListOptions = {
excludeDepartmentId?: string;
enabled?: boolean;
showArchived?: boolean;
};

type DepartmentListItem = {
_id: string;
label: string;
value: string;
selectedDepartment?: string;
};

export const useDepartmentsList = (
Expand All @@ -35,6 +32,7 @@ export const useDepartmentsList = (
const reload = useCallback(() => setItemsList(new RecordList<DepartmentListItem>()), []);

const getDepartments = useEndpoint('GET', '/v1/livechat/department');
const getDepartment = useEndpoint('GET', '/v1/livechat/department/:_id', { _id: options.selectedDepartment ?? '' });

useComponentDidUpdate(() => {
options && reload();
Expand All @@ -60,30 +58,32 @@ export const useDepartmentsList = (
}
return true;
})
.map(({ _id, name, _updatedAt, ...department }): DepartmentListItem => {
return {
.map(
({ _id, name, ...department }): DepartmentListItem => ({
_id,
label: department.archived ? `${name} [${t('Archived')}]` : name,
value: _id,
};
});
}),
);

const normalizedItems = await normalizeDepartments(items, options.selectedDepartment ?? '', getDepartment);

options.haveAll &&
items.unshift({
normalizedItems.unshift({
_id: '',
label: t('All'),
value: 'all',
});

options.haveNone &&
items.unshift({
normalizedItems.unshift({
_id: '',
label: t('None'),
value: '',
});

return {
items,
items: normalizedItems,
itemCount: options.departmentId ? total - 1 : total,
};
},
Expand All @@ -94,9 +94,11 @@ export const useDepartmentsList = (
options.excludeDepartmentId,
options.enabled,
options.showArchived,
options.selectedDepartment,
options.haveAll,
options.haveNone,
options.departmentId,
getDepartment,
t,
],
);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
import type { EndpointFunction } from '@rocket.chat/ui-contexts';

import type { DepartmentListItem } from '../Definitions/DepartmentsDefinitions';

export const normalizeDepartments = async (
departments: DepartmentListItem[],
selectedDepartment: string,
getDepartment: EndpointFunction<'GET', '/v1/livechat/department/:_id'>,
): Promise<DepartmentListItem[]> => {
const isSelectedDepartmentAlreadyOnList = () => departments.some((department) => department._id === selectedDepartment);
if (!selectedDepartment || selectedDepartment === 'all' || isSelectedDepartmentAlreadyOnList()) {
return departments;
}

try {
const { department: missingDepartment } = await getDepartment({});

return missingDepartment
? [...departments, { _id: missingDepartment._id, label: missingDepartment.name, value: missingDepartment._id }]
: departments;
} catch {
return departments;
}
};
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,7 @@ test.describe('OC - Current Chats [Auto Selection]', async () => {
expect(results.violations).toEqual([]);
});

test('OC - Current chats - Filters', async () => {
test('OC - Current chats - Filters', async ({ page }) => {
const [departmentA, departmentB] = departments.map(({ data }) => data);

await test.step('expect to filter by guest', async () => {
Expand Down Expand Up @@ -236,6 +236,12 @@ test.describe('OC - Current Chats [Auto Selection]', async () => {
await expect(poCurrentChats.findRowByName(visitorA)).toBeVisible();
});

await test.step('expect department filter to show selected value after page reload', async () => {
await poCurrentChats.selectDepartment(departmentA.name);
await page.reload();
await expect(poCurrentChats.inputDepartmentValue).toContainText(departmentA.name);
});

// TODO: Unit test await test.step('expect to filter by period', async () => {});

// TODO: Unit test await test.step('expect to filter by custom fields', async () => {});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,10 @@ export class OmnichannelCurrentChats extends OmnichannelAdministration {
return this.page.locator('[data-qa="autocomplete-department"] input');
}

get inputDepartmentValue(): Locator {
return this.page.locator('[data-qa="autocomplete-department"] span');
}

get inputTags(): Locator {
return this.page.locator('[data-qa="current-chats-tags"] [role="listbox"]');
}
Expand Down
Loading