From b05e18065ca1c84282d482e9b99d4dd2dd63287e Mon Sep 17 00:00:00 2001 From: rique223 Date: Fri, 27 Sep 2024 11:03:19 -0300 Subject: [PATCH 01/11] fix: :bug: Blank selected department when selecting out of range MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The the Oc current chats departments filter uses "on scroll" pagination. If a user selects a department that isn’t within the first 25 departments listed, the selection will appear blank after refreshing the page. This happens because the `PaginatedSelectFiltered` component cannot find the selected department in the paginated options list due to it not being part of the initially fetched list of departments. --- .../client/components/AutoCompleteDepartment.tsx | 1 + .../Omnichannel/hooks/useDepartmentsList.ts | 7 ++++++- .../Omnichannel/normalizeDepartments.ts | 16 ++++++++++++++++ 3 files changed, 23 insertions(+), 1 deletion(-) create mode 100644 apps/meteor/client/components/Omnichannel/normalizeDepartments.ts diff --git a/apps/meteor/client/components/AutoCompleteDepartment.tsx b/apps/meteor/client/components/AutoCompleteDepartment.tsx index 0c50f2254aac..5edf008fb3ff 100644 --- a/apps/meteor/client/components/AutoCompleteDepartment.tsx +++ b/apps/meteor/client/components/AutoCompleteDepartment.tsx @@ -42,6 +42,7 @@ const AutoCompleteDepartment = ({ haveNone, excludeDepartmentId, showArchived, + value, }), [debouncedDepartmentsFilter, onlyMyDepartments, haveAll, haveNone, excludeDepartmentId, showArchived], ), diff --git a/apps/meteor/client/components/Omnichannel/hooks/useDepartmentsList.ts b/apps/meteor/client/components/Omnichannel/hooks/useDepartmentsList.ts index fd3c0a29effe..ee68f71f34fa 100644 --- a/apps/meteor/client/components/Omnichannel/hooks/useDepartmentsList.ts +++ b/apps/meteor/client/components/Omnichannel/hooks/useDepartmentsList.ts @@ -4,6 +4,7 @@ import { useCallback, useState } from 'react'; import { useScrollableRecordList } from '../../../hooks/lists/useScrollableRecordList'; import { useComponentDidUpdate } from '../../../hooks/useComponentDidUpdate'; import { RecordList } from '../../../lib/lists/RecordList'; +import { normalizeDepartments } from '../normalizeDepartments'; type DepartmentsListOptions = { filter: string; @@ -14,6 +15,7 @@ type DepartmentsListOptions = { excludeDepartmentId?: string; enabled?: boolean; showArchived?: boolean; + value: string | (string & readonly string[]) | undefined; }; type DepartmentListItem = { @@ -35,6 +37,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.value ?? ''}); useComponentDidUpdate(() => { options && reload(); @@ -53,7 +56,9 @@ export const useDepartmentsList = ( showArchived: options.showArchived ? 'true' : 'false', }); - const items = departments + const normalizedDepartments = await normalizeDepartments(departments, options.value, getDepartment); + + const items = normalizedDepartments .filter((department) => { if (options.departmentId && department._id === options.departmentId) { return false; diff --git a/apps/meteor/client/components/Omnichannel/normalizeDepartments.ts b/apps/meteor/client/components/Omnichannel/normalizeDepartments.ts new file mode 100644 index 000000000000..d4f0cb0b89a7 --- /dev/null +++ b/apps/meteor/client/components/Omnichannel/normalizeDepartments.ts @@ -0,0 +1,16 @@ +import { ILivechatDepartment, Serialized } from '@rocket.chat/core-typings'; +import { EndpointFunction } from '@rocket.chat/ui-contexts'; + +export const normalizeDepartments = async ( + departments: Serialized, + selectedDepartment: string | (string & readonly string[]) | undefined, + getDepartment: EndpointFunction<'GET', '/v1/livechat/department/:_id'>, +) => { + if (selectedDepartment === 'all' || departments.find((department) => department._id === selectedDepartment)) { + return departments; + } + + const { department: missingDepartment } = await getDepartment({}) + + return [...departments, missingDepartment]; +}; From 0e9446853d070582b4b9e6490f31204cf39af8b5 Mon Sep 17 00:00:00 2001 From: rique223 Date: Fri, 27 Sep 2024 12:22:53 -0300 Subject: [PATCH 02/11] typecheck --- .../client/components/Omnichannel/hooks/useDepartmentsList.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/meteor/client/components/Omnichannel/hooks/useDepartmentsList.ts b/apps/meteor/client/components/Omnichannel/hooks/useDepartmentsList.ts index ee68f71f34fa..d28f6b7b2eab 100644 --- a/apps/meteor/client/components/Omnichannel/hooks/useDepartmentsList.ts +++ b/apps/meteor/client/components/Omnichannel/hooks/useDepartmentsList.ts @@ -15,7 +15,7 @@ type DepartmentsListOptions = { excludeDepartmentId?: string; enabled?: boolean; showArchived?: boolean; - value: string | (string & readonly string[]) | undefined; + value?: string | (string & readonly string[]) | undefined; }; type DepartmentListItem = { From 3cc421b835b9933336bd42254d85a7fc5f797aa0 Mon Sep 17 00:00:00 2001 From: rique223 Date: Fri, 27 Sep 2024 13:04:51 -0300 Subject: [PATCH 03/11] code check --- .../client/components/Omnichannel/normalizeDepartments.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/apps/meteor/client/components/Omnichannel/normalizeDepartments.ts b/apps/meteor/client/components/Omnichannel/normalizeDepartments.ts index d4f0cb0b89a7..058cd77299ea 100644 --- a/apps/meteor/client/components/Omnichannel/normalizeDepartments.ts +++ b/apps/meteor/client/components/Omnichannel/normalizeDepartments.ts @@ -1,5 +1,5 @@ -import { ILivechatDepartment, Serialized } from '@rocket.chat/core-typings'; -import { EndpointFunction } from '@rocket.chat/ui-contexts'; +import type { ILivechatDepartment, Serialized } from '@rocket.chat/core-typings'; +import type { EndpointFunction } from '@rocket.chat/ui-contexts'; export const normalizeDepartments = async ( departments: Serialized, @@ -10,7 +10,7 @@ export const normalizeDepartments = async ( return departments; } - const { department: missingDepartment } = await getDepartment({}) + const { department: missingDepartment } = await getDepartment({}); return [...departments, missingDepartment]; }; From 70873a2c800d5f51bbf8c5dedf05a461dfec7196 Mon Sep 17 00:00:00 2001 From: rique223 Date: Fri, 27 Sep 2024 13:05:53 -0300 Subject: [PATCH 04/11] code check 2 --- .../client/components/Omnichannel/hooks/useDepartmentsList.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/meteor/client/components/Omnichannel/hooks/useDepartmentsList.ts b/apps/meteor/client/components/Omnichannel/hooks/useDepartmentsList.ts index d28f6b7b2eab..ec3381040eb0 100644 --- a/apps/meteor/client/components/Omnichannel/hooks/useDepartmentsList.ts +++ b/apps/meteor/client/components/Omnichannel/hooks/useDepartmentsList.ts @@ -37,7 +37,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.value ?? ''}); + const getDepartment = useEndpoint('GET', '/v1/livechat/department/:_id', { _id: options.value ?? '' }); useComponentDidUpdate(() => { options && reload(); From 1decb23b2594f9519e381c7f6963e2e7a5c034d4 Mon Sep 17 00:00:00 2001 From: rique223 Date: Fri, 27 Sep 2024 19:34:07 -0300 Subject: [PATCH 05/11] test: :white_check_mark: Run normalizing function only on AutoCompleteDepartment --- .../client/components/Omnichannel/hooks/useDepartmentsList.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/meteor/client/components/Omnichannel/hooks/useDepartmentsList.ts b/apps/meteor/client/components/Omnichannel/hooks/useDepartmentsList.ts index ec3381040eb0..e39bf07e9c80 100644 --- a/apps/meteor/client/components/Omnichannel/hooks/useDepartmentsList.ts +++ b/apps/meteor/client/components/Omnichannel/hooks/useDepartmentsList.ts @@ -56,7 +56,7 @@ export const useDepartmentsList = ( showArchived: options.showArchived ? 'true' : 'false', }); - const normalizedDepartments = await normalizeDepartments(departments, options.value, getDepartment); + const normalizedDepartments = options.value ? await normalizeDepartments(departments, options.value, getDepartment) : departments; const items = normalizedDepartments .filter((department) => { From 22ce73ac280f588b39d28ffb0a8cb0218478733d Mon Sep 17 00:00:00 2001 From: rique223 Date: Tue, 1 Oct 2024 18:55:00 -0300 Subject: [PATCH 06/11] test: :white_check_mark: useDepartmentsList unit tests and omnichannel current chats e2e tests Added tests to verify when the useDepartmentsList function should normalize the departments list for the current chats department filter. Also added an end-to-end test to validate the use of localStorage in the department filter. --- .../components/AutoCompleteDepartment.tsx | 4 +- .../hooks/useDepartmentsList.spec.ts | 107 ++++++++++++++++++ .../Omnichannel/hooks/useDepartmentsList.ts | 30 ++--- .../Omnichannel/normalizeDepartments.ts | 14 ++- .../omnichannel-current-chats.spec.ts | 8 +- .../page-objects/omnichannel-current-chats.ts | 4 + 6 files changed, 145 insertions(+), 22 deletions(-) create mode 100644 apps/meteor/client/components/Omnichannel/hooks/useDepartmentsList.spec.ts diff --git a/apps/meteor/client/components/AutoCompleteDepartment.tsx b/apps/meteor/client/components/AutoCompleteDepartment.tsx index 5edf008fb3ff..67f96f4a414c 100644 --- a/apps/meteor/client/components/AutoCompleteDepartment.tsx +++ b/apps/meteor/client/components/AutoCompleteDepartment.tsx @@ -42,9 +42,9 @@ const AutoCompleteDepartment = ({ haveNone, excludeDepartmentId, showArchived, - value, + selectedDepartment: value, }), - [debouncedDepartmentsFilter, onlyMyDepartments, haveAll, haveNone, excludeDepartmentId, showArchived], + [debouncedDepartmentsFilter, onlyMyDepartments, haveAll, haveNone, excludeDepartmentId, showArchived, value], ), ); 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..86bb71f66401 --- /dev/null +++ b/apps/meteor/client/components/Omnichannel/hooks/useDepartmentsList.spec.ts @@ -0,0 +1,107 @@ +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 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 { 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, + })) + .build(), + }, + ); + + 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 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 e39bf07e9c80..d6b0de745e83 100644 --- a/apps/meteor/client/components/Omnichannel/hooks/useDepartmentsList.ts +++ b/apps/meteor/client/components/Omnichannel/hooks/useDepartmentsList.ts @@ -15,10 +15,10 @@ type DepartmentsListOptions = { excludeDepartmentId?: string; enabled?: boolean; showArchived?: boolean; - value?: string | (string & readonly string[]) | undefined; + selectedDepartment?: string; }; -type DepartmentListItem = { +export type DepartmentListItem = { _id: string; label: string; value: string; @@ -37,7 +37,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.value ?? '' }); + const getDepartment = useEndpoint('GET', '/v1/livechat/department/:_id', { _id: options.selectedDepartment ?? '' }); useComponentDidUpdate(() => { options && reload(); @@ -56,39 +56,39 @@ export const useDepartmentsList = ( showArchived: options.showArchived ? 'true' : 'false', }); - const normalizedDepartments = options.value ? await normalizeDepartments(departments, options.value, getDepartment) : departments; - - const items = normalizedDepartments + const items = departments .filter((department) => { if (options.departmentId && department._id === options.departmentId) { return false; } 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, }; }, @@ -99,14 +99,16 @@ export const useDepartmentsList = ( options.excludeDepartmentId, options.enabled, options.showArchived, + options.selectedDepartment, options.haveAll, options.haveNone, options.departmentId, + getDepartment, t, ], ); - const { loadMoreItems, initialItemCount } = useScrollableRecordList(itemsList, fetchData, 25); + const { loadMoreItems, initialItemCount } = useScrollableRecordList(itemsList, fetchData, 5); return { reload, diff --git a/apps/meteor/client/components/Omnichannel/normalizeDepartments.ts b/apps/meteor/client/components/Omnichannel/normalizeDepartments.ts index 058cd77299ea..1ccfbee076a6 100644 --- a/apps/meteor/client/components/Omnichannel/normalizeDepartments.ts +++ b/apps/meteor/client/components/Omnichannel/normalizeDepartments.ts @@ -1,16 +1,20 @@ -import type { ILivechatDepartment, Serialized } from '@rocket.chat/core-typings'; import type { EndpointFunction } from '@rocket.chat/ui-contexts'; +import type { DepartmentListItem } from './hooks/useDepartmentsList'; + export const normalizeDepartments = async ( - departments: Serialized, - selectedDepartment: string | (string & readonly string[]) | undefined, + departments: DepartmentListItem[], + selectedDepartment: string, getDepartment: EndpointFunction<'GET', '/v1/livechat/department/:_id'>, ) => { - if (selectedDepartment === 'all' || departments.find((department) => department._id === selectedDepartment)) { + const isSelectedDepartmentAlreadyOnList = departments.find((department) => department._id === selectedDepartment); + if (!selectedDepartment || selectedDepartment === 'all' || isSelectedDepartmentAlreadyOnList) { return departments; } const { department: missingDepartment } = await getDepartment({}); - return [...departments, missingDepartment]; + return missingDepartment + ? [...departments, { _id: missingDepartment._id, label: missingDepartment.name, value: missingDepartment._id }] + : 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..16fce4ba9146 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"]'); } From 408e808af2409f3053fb87de1a37de1cf781fc2a Mon Sep 17 00:00:00 2001 From: rique223 Date: Tue, 1 Oct 2024 19:20:30 -0300 Subject: [PATCH 07/11] Code check --- .../tests/e2e/omnichannel/omnichannel-current-chats.spec.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 16fce4ba9146..8e121b36ddb0 100644 --- a/apps/meteor/tests/e2e/omnichannel/omnichannel-current-chats.spec.ts +++ b/apps/meteor/tests/e2e/omnichannel/omnichannel-current-chats.spec.ts @@ -236,7 +236,7 @@ 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 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); From 9315d9c644900f5c770a020d00fbbd19d3377664 Mon Sep 17 00:00:00 2001 From: rique223 Date: Wed, 2 Oct 2024 16:31:18 -0300 Subject: [PATCH 08/11] fix: :bug: Re-add default pagination to use useScrollableRecordList --- .../client/components/Omnichannel/hooks/useDepartmentsList.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/meteor/client/components/Omnichannel/hooks/useDepartmentsList.ts b/apps/meteor/client/components/Omnichannel/hooks/useDepartmentsList.ts index d6b0de745e83..3ed33b56491f 100644 --- a/apps/meteor/client/components/Omnichannel/hooks/useDepartmentsList.ts +++ b/apps/meteor/client/components/Omnichannel/hooks/useDepartmentsList.ts @@ -108,7 +108,7 @@ export const useDepartmentsList = ( ], ); - const { loadMoreItems, initialItemCount } = useScrollableRecordList(itemsList, fetchData, 5); + const { loadMoreItems, initialItemCount } = useScrollableRecordList(itemsList, fetchData, 25); return { reload, From f8d670fc8d637e358b068dc959ad41ac8d2ed4ea Mon Sep 17 00:00:00 2001 From: rique223 Date: Mon, 7 Oct 2024 14:20:27 -0300 Subject: [PATCH 09/11] Reviews --- .../Omnichannel/hooks/useDepartmentsList.spec.ts | 8 ++++++-- .../components/Omnichannel/hooks/useDepartmentsList.ts | 2 +- .../Omnichannel/{ => utils}/normalizeDepartments.ts | 8 ++++---- 3 files changed, 11 insertions(+), 7 deletions(-) rename apps/meteor/client/components/Omnichannel/{ => utils}/normalizeDepartments.ts (68%) diff --git a/apps/meteor/client/components/Omnichannel/hooks/useDepartmentsList.spec.ts b/apps/meteor/client/components/Omnichannel/hooks/useDepartmentsList.spec.ts index 86bb71f66401..173677799b08 100644 --- a/apps/meteor/client/components/Omnichannel/hooks/useDepartmentsList.spec.ts +++ b/apps/meteor/client/components/Omnichannel/hooks/useDepartmentsList.spec.ts @@ -20,13 +20,15 @@ const initialDepartmentsListMock = Array.from(Array(25)).map((_, index) => { }; }); -it('should not add selected department if it is already in the departments list on first fetch', async () => { +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({ @@ -45,16 +47,18 @@ it('should not add selected department if it is already in the departments list 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 add selected department if it is not part of departments list on first fetch', async () => { +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', diff --git a/apps/meteor/client/components/Omnichannel/hooks/useDepartmentsList.ts b/apps/meteor/client/components/Omnichannel/hooks/useDepartmentsList.ts index 3ed33b56491f..f251c453c4d0 100644 --- a/apps/meteor/client/components/Omnichannel/hooks/useDepartmentsList.ts +++ b/apps/meteor/client/components/Omnichannel/hooks/useDepartmentsList.ts @@ -4,7 +4,7 @@ import { useCallback, useState } from 'react'; import { useScrollableRecordList } from '../../../hooks/lists/useScrollableRecordList'; import { useComponentDidUpdate } from '../../../hooks/useComponentDidUpdate'; import { RecordList } from '../../../lib/lists/RecordList'; -import { normalizeDepartments } from '../normalizeDepartments'; +import { normalizeDepartments } from '../utils/normalizeDepartments'; type DepartmentsListOptions = { filter: string; diff --git a/apps/meteor/client/components/Omnichannel/normalizeDepartments.ts b/apps/meteor/client/components/Omnichannel/utils/normalizeDepartments.ts similarity index 68% rename from apps/meteor/client/components/Omnichannel/normalizeDepartments.ts rename to apps/meteor/client/components/Omnichannel/utils/normalizeDepartments.ts index 1ccfbee076a6..13cbe69eaf2d 100644 --- a/apps/meteor/client/components/Omnichannel/normalizeDepartments.ts +++ b/apps/meteor/client/components/Omnichannel/utils/normalizeDepartments.ts @@ -1,14 +1,14 @@ import type { EndpointFunction } from '@rocket.chat/ui-contexts'; -import type { DepartmentListItem } from './hooks/useDepartmentsList'; +import type { DepartmentListItem } from '../hooks/useDepartmentsList'; export const normalizeDepartments = async ( departments: DepartmentListItem[], selectedDepartment: string, getDepartment: EndpointFunction<'GET', '/v1/livechat/department/:_id'>, -) => { - const isSelectedDepartmentAlreadyOnList = departments.find((department) => department._id === selectedDepartment); - if (!selectedDepartment || selectedDepartment === 'all' || isSelectedDepartmentAlreadyOnList) { +): Promise => { + const isSelectedDepartmentAlreadyOnList = () => departments.some((department) => department._id === selectedDepartment); + if (!selectedDepartment || selectedDepartment === 'all' || isSelectedDepartmentAlreadyOnList()) { return departments; } From 1c2b390cfaf4e4ce95e9622055122e01b43db52f Mon Sep 17 00:00:00 2001 From: rique223 Date: Mon, 7 Oct 2024 18:24:33 -0300 Subject: [PATCH 10/11] Reviews 2 --- .../Definitions/DepartmentsDefinitions.ts | 5 +++++ .../Omnichannel/hooks/useDepartmentsList.ts | 7 +------ .../Omnichannel/utils/normalizeDepartments.ts | 14 +++++++++----- 3 files changed, 15 insertions(+), 11 deletions(-) create mode 100644 apps/meteor/client/components/Omnichannel/Definitions/DepartmentsDefinitions.ts 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.ts b/apps/meteor/client/components/Omnichannel/hooks/useDepartmentsList.ts index f251c453c4d0..d8e1071bf509 100644 --- a/apps/meteor/client/components/Omnichannel/hooks/useDepartmentsList.ts +++ b/apps/meteor/client/components/Omnichannel/hooks/useDepartmentsList.ts @@ -4,6 +4,7 @@ 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 = { @@ -18,12 +19,6 @@ type DepartmentsListOptions = { selectedDepartment?: string; }; -export type DepartmentListItem = { - _id: string; - label: string; - value: string; -}; - export const useDepartmentsList = ( options: DepartmentsListOptions, ): { diff --git a/apps/meteor/client/components/Omnichannel/utils/normalizeDepartments.ts b/apps/meteor/client/components/Omnichannel/utils/normalizeDepartments.ts index 13cbe69eaf2d..6c27db246b1d 100644 --- a/apps/meteor/client/components/Omnichannel/utils/normalizeDepartments.ts +++ b/apps/meteor/client/components/Omnichannel/utils/normalizeDepartments.ts @@ -1,6 +1,6 @@ import type { EndpointFunction } from '@rocket.chat/ui-contexts'; -import type { DepartmentListItem } from '../hooks/useDepartmentsList'; +import type { DepartmentListItem } from '../Definitions/DepartmentsDefinitions'; export const normalizeDepartments = async ( departments: DepartmentListItem[], @@ -12,9 +12,13 @@ export const normalizeDepartments = async ( return departments; } - const { department: missingDepartment } = await getDepartment({}); + try { + const { department: missingDepartment } = await getDepartment({}); - return missingDepartment - ? [...departments, { _id: missingDepartment._id, label: missingDepartment.name, value: missingDepartment._id }] - : departments; + return missingDepartment + ? [...departments, { _id: missingDepartment._id, label: missingDepartment.name, value: missingDepartment._id }] + : departments; + } catch { + return departments; + } }; From 9f27c716b8dee828de4ae69517c18a757136c694 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrique=20Guimar=C3=A3es=20Ribeiro?= <43561537+rique223@users.noreply.github.com> Date: Tue, 8 Oct 2024 11:37:05 -0300 Subject: [PATCH 11/11] Create sharp-adults-think.md --- .changeset/sharp-adults-think.md | 7 +++++++ 1 file changed, 7 insertions(+) create mode 100644 .changeset/sharp-adults-think.md 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.