From dcc49190d6809780f49e760da01a268d2d86e9a3 Mon Sep 17 00:00:00 2001 From: Navin Karkera Date: Wed, 7 Aug 2024 15:54:17 +0530 Subject: [PATCH 1/4] fix: discard changes --- src/library-authoring/data/apiHooks.ts | 1 + .../library-info/LibraryInfo.test.tsx | 26 +++++++++++++++++++ .../library-info/LibraryPublishStatus.tsx | 21 +++++---------- 3 files changed, 34 insertions(+), 14 deletions(-) diff --git a/src/library-authoring/data/apiHooks.ts b/src/library-authoring/data/apiHooks.ts index c09be62c91..c7748f5e93 100644 --- a/src/library-authoring/data/apiHooks.ts +++ b/src/library-authoring/data/apiHooks.ts @@ -121,6 +121,7 @@ export const useRevertLibraryChanges = () => { mutationFn: revertLibraryChanges, onSettled: (_data, _error, libraryId) => { queryClient.invalidateQueries({ queryKey: libraryAuthoringQueryKeys.contentLibrary(libraryId) }); + queryClient.invalidateQueries({ queryKey: ['content_search'] }); }, }); }; diff --git a/src/library-authoring/library-info/LibraryInfo.test.tsx b/src/library-authoring/library-info/LibraryInfo.test.tsx index 5ace15eb94..48c396ab58 100644 --- a/src/library-authoring/library-info/LibraryInfo.test.tsx +++ b/src/library-authoring/library-info/LibraryInfo.test.tsx @@ -204,4 +204,30 @@ describe('', () => { await waitFor(() => expect(axiosMock.history.post[0].url).toEqual(url)); }); + + it('should discard changes', async () => { + const url = getCommitLibraryChangesUrl(libraryData.id); + axiosMock.onDelete(url).reply(200); + + render(); + const discardButton = screen.getByRole('button', { name: /discard changes/i }); + fireEvent.click(discardButton); + + expect(await screen.findByText('Library changes reverted successfully')).toBeInTheDocument(); + + await waitFor(() => expect(axiosMock.history.delete[0].url).toEqual(url)); + }); + + it('should show error on discard changes', async () => { + const url = getCommitLibraryChangesUrl(libraryData.id); + axiosMock.onDelete(url).reply(500); + + render(); + const discardButton = screen.getByRole('button', { name: /discard changes/i }); + fireEvent.click(discardButton); + + expect(await screen.findByText('There was an error reverting changes in the library.')).toBeInTheDocument(); + + await waitFor(() => expect(axiosMock.history.delete[0].url).toEqual(url)); + }); }); diff --git a/src/library-authoring/library-info/LibraryPublishStatus.tsx b/src/library-authoring/library-info/LibraryPublishStatus.tsx index e497042657..d159d641cb 100644 --- a/src/library-authoring/library-info/LibraryPublishStatus.tsx +++ b/src/library-authoring/library-info/LibraryPublishStatus.tsx @@ -2,7 +2,7 @@ import React, { useCallback, useContext, useMemo } from 'react'; import classNames from 'classnames'; import { Button, Container, Stack } from '@openedx/paragon'; import { FormattedDate, FormattedTime, useIntl } from '@edx/frontend-platform/i18n'; -import { useCommitLibraryChanges } from '../data/apiHooks'; +import { useCommitLibraryChanges, useRevertLibraryChanges } from '../data/apiHooks'; import { ContentLibrary } from '../data/api'; import { ToastContext } from '../../generic/toast-context'; import messages from './messages'; @@ -14,6 +14,7 @@ type LibraryPublishStatusProps = { const LibraryPublishStatus = ({ library } : LibraryPublishStatusProps) => { const intl = useIntl(); const commitLibraryChanges = useCommitLibraryChanges(); + const revertLibraryChanges = useRevertLibraryChanges(); const { showToast } = useContext(ToastContext); const commit = useCallback(() => { @@ -25,9 +26,6 @@ const LibraryPublishStatus = ({ library } : LibraryPublishStatusProps) => { }); }, []); - /** - * TODO, the discard changes breaks the library. - * Discomment this when discard changes is fixed. const revert = useCallback(() => { revertLibraryChanges.mutateAsync(library.id) .then(() => { @@ -36,7 +34,6 @@ const LibraryPublishStatus = ({ library } : LibraryPublishStatusProps) => { showToast(intl.formatMessage(messages.revertErrorMsg)); }); }, []); - */ const { isPublished, @@ -153,15 +150,11 @@ const LibraryPublishStatus = ({ library } : LibraryPublishStatusProps) => { - { /* - * TODO, the discard changes breaks the library. - * Discomment this when discard changes is fixed. -
- -
- */ } +
+ +
From bb3642458cbd0f0880ec73dc2f2493da65b70eba Mon Sep 17 00:00:00 2001 From: Navin Karkera Date: Wed, 7 Aug 2024 19:44:54 +0530 Subject: [PATCH 2/4] refactor: disable discard btn for new libs Enable it if components are added. --- .../library-info/LibraryInfo.test.tsx | 14 ++++++++++++++ .../library-info/LibraryPublishStatus.tsx | 6 +++++- 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/src/library-authoring/library-info/LibraryInfo.test.tsx b/src/library-authoring/library-info/LibraryInfo.test.tsx index 48c396ab58..09c8350e08 100644 --- a/src/library-authoring/library-info/LibraryInfo.test.tsx +++ b/src/library-authoring/library-info/LibraryInfo.test.tsx @@ -230,4 +230,18 @@ describe('', () => { await waitFor(() => expect(axiosMock.history.delete[0].url).toEqual(url)); }); + + it('discard changes btn should be disabled for new libraries', async () => { + render(); + const discardButton = screen.getByRole('button', { name: /discard changes/i }); + + expect(discardButton).toBeDisabled(); + }); + + it('discard changes btn should be enabled for new libraries if components are added', async () => { + render(); + const discardButton = screen.getByRole('button', { name: /discard changes/i }); + + expect(discardButton).not.toBeDisabled(); + }); }); diff --git a/src/library-authoring/library-info/LibraryPublishStatus.tsx b/src/library-authoring/library-info/LibraryPublishStatus.tsx index d159d641cb..a45ddccf66 100644 --- a/src/library-authoring/library-info/LibraryPublishStatus.tsx +++ b/src/library-authoring/library-info/LibraryPublishStatus.tsx @@ -37,11 +37,13 @@ const LibraryPublishStatus = ({ library } : LibraryPublishStatusProps) => { const { isPublished, + isNew, statusMessage, extraStatusMessage, bodyMessage, } = useMemo(() => { let isPublishedResult: boolean; + let isNewResult = false; let statusMessageResult : string; let extraStatusMessageResult : string | undefined; let bodyMessageResult : string | undefined; @@ -91,6 +93,7 @@ const LibraryPublishStatus = ({ library } : LibraryPublishStatusProps) => { if (!library.lastPublished) { // Library is never published (new) + isNewResult = library.numBlocks === 0; // allow discarding if components are added isPublishedResult = false; statusMessageResult = intl.formatMessage(messages.draftStatusLabel); extraStatusMessageResult = intl.formatMessage(messages.neverPublishedLabel); @@ -120,6 +123,7 @@ const LibraryPublishStatus = ({ library } : LibraryPublishStatusProps) => { } return { isPublished: isPublishedResult, + isNew: isNewResult, statusMessage: statusMessageResult, extraStatusMessage: extraStatusMessageResult, bodyMessage: bodyMessageResult, @@ -151,7 +155,7 @@ const LibraryPublishStatus = ({ library } : LibraryPublishStatusProps) => { {intl.formatMessage(messages.publishButtonLabel)}
-
From 08452e6f9118a453351155224a4309d0d000ed2d Mon Sep 17 00:00:00 2001 From: Navin Karkera Date: Wed, 14 Aug 2024 15:42:43 +0530 Subject: [PATCH 3/4] refactor: invalidate library related content queries --- src/library-authoring/data/apiHooks.ts | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/src/library-authoring/data/apiHooks.ts b/src/library-authoring/data/apiHooks.ts index c7748f5e93..35ab3b3c8c 100644 --- a/src/library-authoring/data/apiHooks.ts +++ b/src/library-authoring/data/apiHooks.ts @@ -1,4 +1,6 @@ -import { useQuery, useMutation, useQueryClient } from '@tanstack/react-query'; +import { + useQuery, useMutation, useQueryClient, Query, +} from '@tanstack/react-query'; import { type GetLibrariesV2CustomParams, @@ -121,7 +123,17 @@ export const useRevertLibraryChanges = () => { mutationFn: revertLibraryChanges, onSettled: (_data, _error, libraryId) => { queryClient.invalidateQueries({ queryKey: libraryAuthoringQueryKeys.contentLibrary(libraryId) }); - queryClient.invalidateQueries({ queryKey: ['content_search'] }); + queryClient.invalidateQueries({ + // Invalidate all content queries related to this library + predicate: /* istanbul ignore next */ (query: Query): boolean => { + // extraFilter contains library id + const extraFilter = query.queryKey[5]; + if (!(Array.isArray(extraFilter) || typeof extraFilter === 'string')) { + return false; + } + return query.queryKey[0] === 'content_search' && extraFilter?.includes(`context_key = "${libraryId}"`); + }, + }); }, }); }; From 2b061f2e9f536c70c4fbd0c996e7f3eff01ec0df Mon Sep 17 00:00:00 2001 From: Navin Karkera Date: Wed, 14 Aug 2024 20:58:39 +0530 Subject: [PATCH 4/4] chore: add comment about content search query invalidation --- src/library-authoring/data/apiHooks.ts | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/library-authoring/data/apiHooks.ts b/src/library-authoring/data/apiHooks.ts index 35ab3b3c8c..a59d1655fb 100644 --- a/src/library-authoring/data/apiHooks.ts +++ b/src/library-authoring/data/apiHooks.ts @@ -124,7 +124,12 @@ export const useRevertLibraryChanges = () => { onSettled: (_data, _error, libraryId) => { queryClient.invalidateQueries({ queryKey: libraryAuthoringQueryKeys.contentLibrary(libraryId) }); queryClient.invalidateQueries({ - // Invalidate all content queries related to this library + // Invalidate all content queries related to this library. + // If we allow searching "all courses and libraries" in the future, + // then we'd have to invalidate all `["content_search", "results"]` + // queries, and not just the ones for this library, because items from + // this library could be included in an "all courses and libraries" + // search. For now we only allow searching individual libraries. predicate: /* istanbul ignore next */ (query: Query): boolean => { // extraFilter contains library id const extraFilter = query.queryKey[5];