From 678faed48b14aebf48a578c402294046e542d51f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrique=20Guimar=C3=A3es=20Ribeiro?= <43561537+rique223@users.noreply.github.com> Date: Wed, 9 Oct 2024 13:58:34 -0300 Subject: [PATCH] fix: Blank selected department when selecting out of range (#33386) --- .changeset/sharp-adults-think.md | 7 ++ .../components/AutoCompleteDepartment.tsx | 3 +- .../Definitions/DepartmentsDefinitions.ts | 5 + .../hooks/useDepartmentsList.spec.ts | 111 ++++++++++++++++++ .../Omnichannel/hooks/useDepartmentsList.ts | 28 +++-- .../Omnichannel/utils/normalizeDepartments.ts | 24 ++++ .../omnichannel-current-chats.spec.ts | 8 +- .../page-objects/omnichannel-current-chats.ts | 4 + 8 files changed, 175 insertions(+), 15 deletions(-) create mode 100644 .changeset/sharp-adults-think.md create mode 100644 apps/meteor/client/components/Omnichannel/Definitions/DepartmentsDefinitions.ts create mode 100644 apps/meteor/client/components/Omnichannel/hooks/useDepartmentsList.spec.ts create mode 100644 apps/meteor/client/components/Omnichannel/utils/normalizeDepartments.ts diff --git a/.changeset/sharp-adults-think.md b/.changeset/sharp-adults-think.md new file mode 100644 index 000000000000..399d3bae1ce2 --- /dev/null +++ b/.changeset/sharp-adults-think.md @@ -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. diff --git a/apps/meteor/client/components/AutoCompleteDepartment.tsx b/apps/meteor/client/components/AutoCompleteDepartment.tsx index 6217f1d99610..7ff38c9c3ca4 100644 --- a/apps/meteor/client/components/AutoCompleteDepartment.tsx +++ b/apps/meteor/client/components/AutoCompleteDepartment.tsx @@ -42,8 +42,9 @@ const AutoCompleteDepartment = ({ haveNone, excludeDepartmentId, showArchived, + selectedDepartment: value, }), - [debouncedDepartmentsFilter, onlyMyDepartments, haveAll, haveNone, excludeDepartmentId, showArchived], + [debouncedDepartmentsFilter, onlyMyDepartments, haveAll, haveNone, excludeDepartmentId, showArchived, value], ), ); diff --git a/apps/meteor/client/components/Omnichannel/Definitions/DepartmentsDefinitions.ts b/apps/meteor/client/components/Omnichannel/Definitions/DepartmentsDefinitions.ts new file mode 100644 index 000000000000..8c6a2301bd66 --- /dev/null +++ b/apps/meteor/client/components/Omnichannel/Definitions/DepartmentsDefinitions.ts @@ -0,0 +1,5 @@ +export type DepartmentListItem = { + _id: string; + label: string; + value: string; +}; diff --git a/apps/meteor/client/components/Omnichannel/hooks/useDepartmentsList.spec.ts b/apps/meteor/client/components/Omnichannel/hooks/useDepartmentsList.spec.ts new file mode 100644 index 000000000000..173677799b08 --- /dev/null +++ b/apps/meteor/client/components/Omnichannel/hooks/useDepartmentsList.spec.ts @@ -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)); +}); diff --git a/apps/meteor/client/components/Omnichannel/hooks/useDepartmentsList.ts b/apps/meteor/client/components/Omnichannel/hooks/useDepartmentsList.ts index fd3c0a29effe..d8e1071bf509 100644 --- a/apps/meteor/client/components/Omnichannel/hooks/useDepartmentsList.ts +++ b/apps/meteor/client/components/Omnichannel/hooks/useDepartmentsList.ts @@ -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; @@ -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 = ( @@ -35,6 +32,7 @@ export const useDepartmentsList = ( const reload = useCallback(() => setItemsList(new RecordList()), []); const getDepartments = useEndpoint('GET', '/v1/livechat/department'); + const getDepartment = useEndpoint('GET', '/v1/livechat/department/:_id', { _id: options.selectedDepartment ?? '' }); useComponentDidUpdate(() => { options && reload(); @@ -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, }; }, @@ -94,9 +94,11 @@ export const useDepartmentsList = ( options.excludeDepartmentId, options.enabled, options.showArchived, + options.selectedDepartment, options.haveAll, options.haveNone, options.departmentId, + getDepartment, t, ], ); diff --git a/apps/meteor/client/components/Omnichannel/utils/normalizeDepartments.ts b/apps/meteor/client/components/Omnichannel/utils/normalizeDepartments.ts new file mode 100644 index 000000000000..6c27db246b1d --- /dev/null +++ b/apps/meteor/client/components/Omnichannel/utils/normalizeDepartments.ts @@ -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 => { + 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; + } +}; diff --git a/apps/meteor/tests/e2e/omnichannel/omnichannel-current-chats.spec.ts b/apps/meteor/tests/e2e/omnichannel/omnichannel-current-chats.spec.ts index be8ec38a49aa..8e121b36ddb0 100644 --- a/apps/meteor/tests/e2e/omnichannel/omnichannel-current-chats.spec.ts +++ b/apps/meteor/tests/e2e/omnichannel/omnichannel-current-chats.spec.ts @@ -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 () => { @@ -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 () => {}); diff --git a/apps/meteor/tests/e2e/page-objects/omnichannel-current-chats.ts b/apps/meteor/tests/e2e/page-objects/omnichannel-current-chats.ts index 46773e0ca43a..87a21cf34c8f 100644 --- a/apps/meteor/tests/e2e/page-objects/omnichannel-current-chats.ts +++ b/apps/meteor/tests/e2e/page-objects/omnichannel-current-chats.ts @@ -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"]'); }