From b8c7c3d1e4eabaf10387128a4bc4c42d6dd78cf4 Mon Sep 17 00:00:00 2001 From: Anthony LC Date: Thu, 9 Nov 2023 17:00:48 +0100 Subject: [PATCH] =?UTF-8?q?=F0=9F=90=9B(classroom)=20improve=20data=20upda?= =?UTF-8?q?te=20on=20ToolsAndApplications?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - If the user was a bit slow to write in the recording purpose textarea, his input was overwritten by his own recording. - Only the last update was sent to the backend, so if you were updated 2 toggles quickly, only the last one was updated in backend --- .../ToolsAndApplications/index.spec.tsx | 82 ++++++++++++++++--- .../widgets/ToolsAndApplications/index.tsx | 39 ++++++--- 2 files changed, 97 insertions(+), 24 deletions(-) diff --git a/src/frontend/packages/lib_classroom/src/components/ClassroomWidgetProvider/widgets/ToolsAndApplications/index.spec.tsx b/src/frontend/packages/lib_classroom/src/components/ClassroomWidgetProvider/widgets/ToolsAndApplications/index.spec.tsx index 8645cd8e7f..679f89368a 100644 --- a/src/frontend/packages/lib_classroom/src/components/ClassroomWidgetProvider/widgets/ToolsAndApplications/index.spec.tsx +++ b/src/frontend/packages/lib_classroom/src/components/ClassroomWidgetProvider/widgets/ToolsAndApplications/index.spec.tsx @@ -69,14 +69,17 @@ describe('', () => { expect(toggleEnableChat).toBeChecked(); await userEvent.click(toggleEnableChat); - expect(await screen.findByText('Classroom updated.')).toBeInTheDocument(); + expect( + await screen.findByText('Classroom updated.', {}, { timeout: 2000 }), + ).toBeInTheDocument(); + expect(fetchMock.lastCall()![1]).toEqual({ headers: { 'Content-Type': 'application/json', 'Accept-Language': 'en', }, method: 'PATCH', - body: '{"enable_chat":false}', + body: expect.stringContaining(`"enable_chat":false`), }); }); @@ -99,14 +102,17 @@ describe('', () => { expect(toggleEnableSharedNotes).toBeChecked(); await userEvent.click(toggleEnableSharedNotes); - expect(await screen.findByText('Classroom updated.')).toBeInTheDocument(); + expect( + await screen.findByText('Classroom updated.', {}, { timeout: 2000 }), + ).toBeInTheDocument(); + expect(fetchMock.lastCall()![1]).toEqual({ headers: { 'Content-Type': 'application/json', 'Accept-Language': 'en', }, method: 'PATCH', - body: '{"enable_shared_notes":false}', + body: expect.stringContaining(`"enable_shared_notes":false`), }); }); @@ -129,14 +135,17 @@ describe('', () => { expect(toggleEnableWaitingRoom).not.toBeChecked(); await userEvent.click(toggleEnableWaitingRoom); - expect(await screen.findByText('Classroom updated.')).toBeInTheDocument(); + expect( + await screen.findByText('Classroom updated.', {}, { timeout: 2000 }), + ).toBeInTheDocument(); + expect(fetchMock.lastCall()![1]).toEqual({ headers: { 'Content-Type': 'application/json', 'Accept-Language': 'en', }, method: 'PATCH', - body: '{"enable_waiting_room":true}', + body: expect.stringContaining(`"enable_waiting_room":true`), }); }); @@ -174,14 +183,17 @@ describe('', () => { await userEvent.click(toggleEnableRecordings); - expect(await screen.findByText('Classroom updated.')).toBeInTheDocument(); + expect( + await screen.findByText('Classroom updated.', {}, { timeout: 2000 }), + ).toBeInTheDocument(); + expect(fetchMock.lastCall()![1]).toEqual({ headers: { 'Content-Type': 'application/json', 'Accept-Language': 'en', }, method: 'PATCH', - body: '{"enable_recordings":false}', + body: expect.stringContaining(`"enable_recordings":false`), }); await waitFor(() => { @@ -231,14 +243,19 @@ describe('', () => { recording_purpose: recording_purpose_updated, }); - expect(await screen.findByText('Classroom updated.')).toBeInTheDocument(); + expect( + await screen.findByText('Classroom updated.', {}, { timeout: 2000 }), + ).toBeInTheDocument(); + expect(fetchMock.lastCall()![1]).toEqual({ headers: { 'Content-Type': 'application/json', 'Accept-Language': 'en', }, method: 'PATCH', - body: `{"recording_purpose":"${recording_purpose_updated}"}`, + body: expect.stringContaining( + `"recording_purpose":"${recording_purpose_updated}"`, + ), }); expect(screen.getByText(recording_purpose_updated)).toBeInTheDocument(); @@ -262,8 +279,51 @@ describe('', () => { await userEvent.click(toggleEnableRecordings); expect( - await screen.findByText('Classroom not updated!'), + await screen.findByText('Classroom not updated!', {}, { timeout: 2000 }), ).toBeInTheDocument(); + expect(toggleEnableRecordings).toBeChecked(); }); + + it('can update multiple properties in one http call', async () => { + const classroom = classroomMockFactory({ id: '1', started: false }); + mockedUseCurrentClassroom.mockReturnValue(classroom); + fetchMock.patch('/api/classrooms/1/', { + ...classroom, + enable_waiting_room: true, + enable_shared_notes: false, + }); + render( + + + , + ); + + const toggleEnableWaitingRoom = screen.getByRole('checkbox', { + name: 'Enable waiting room', + }); + expect(toggleEnableWaitingRoom).not.toBeChecked(); + await userEvent.click(toggleEnableWaitingRoom); + + const toggleEnableSharedNotes = screen.getByRole('checkbox', { + name: 'Enable shared notes', + }); + expect(toggleEnableSharedNotes).toBeChecked(); + await userEvent.click(toggleEnableSharedNotes); + + expect( + await screen.findByText('Classroom updated.', {}, { timeout: 2000 }), + ).toBeInTheDocument(); + + expect(fetchMock.lastCall()![1]).toEqual({ + headers: { + 'Content-Type': 'application/json', + 'Accept-Language': 'en', + }, + method: 'PATCH', + body: + expect.stringContaining(`"enable_waiting_room":true`) && + expect.stringContaining(`"enable_shared_notes":false`), + }); + }); }); diff --git a/src/frontend/packages/lib_classroom/src/components/ClassroomWidgetProvider/widgets/ToolsAndApplications/index.tsx b/src/frontend/packages/lib_classroom/src/components/ClassroomWidgetProvider/widgets/ToolsAndApplications/index.tsx index 5f20ea2d64..40201a0380 100644 --- a/src/frontend/packages/lib_classroom/src/components/ClassroomWidgetProvider/widgets/ToolsAndApplications/index.tsx +++ b/src/frontend/packages/lib_classroom/src/components/ClassroomWidgetProvider/widgets/ToolsAndApplications/index.tsx @@ -82,6 +82,7 @@ const ToolsAndApplicationCheckbox = ({ export const ToolsAndApplications = () => { const classroom = useCurrentClassroom(); + const descriptionInit = useRef(classroom.description); const intl = useIntl(); const updateClassroomMutation = useUpdateClassroom(classroom.id, { onSuccess: () => { @@ -97,24 +98,36 @@ export const ToolsAndApplications = () => { const timeoutRef = useRef(); const handleChange = (updatedClassroomAttribute: Partial) => { - const timeout = 1000; + const timeout = 1500; - setUpdatedClassroomState({ - ...updatedClassroomState, - ...updatedClassroomAttribute, - }); + setUpdatedClassroomState((_updatedClassroomState) => { + const updatedClassroom = { + ..._updatedClassroomState, + ...updatedClassroomAttribute, + }; - if (timeoutRef.current) { - clearTimeout(timeoutRef.current); - } - timeoutRef.current = setTimeout(() => { - updateClassroomMutation.mutate(updatedClassroomAttribute); - }, timeout); + if (timeoutRef.current) { + clearTimeout(timeoutRef.current); + } + + timeoutRef.current = setTimeout(() => { + updateClassroomMutation.mutate(updatedClassroom); + }, timeout); + + return updatedClassroom; + }); }; useEffect(() => { - setUpdatedClassroomState(classroom); - }, [classroom]); + const isIdle = + descriptionInit.current === updatedClassroomState.description; + const isWriting = + updatedClassroomState.description !== classroom.description; + if (isIdle || !isWriting) { + setUpdatedClassroomState(classroom); + descriptionInit.current = classroom.description; + } + }, [updatedClassroomState.description, classroom]); return (