fix: discard changes [FC-0062] - #58
Conversation
Enable it if components are added.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## chris/FAL-3760-library-info-sidebar #58 +/- ##
======================================================================
Coverage ? 92.97%
======================================================================
Files ? 756
Lines ? 13628
Branches ? 2946
======================================================================
Hits ? 12670
Misses ? 922
Partials ? 36 ☔ View full report in Codecov by Sentry. |
ChrisChV
left a comment
There was a problem hiding this comment.
@navinkarkera Looks good, great work 👍
- I tested this: I followed the testing instructions. I tested with a new library, a tested after publish a library and I tested the discard changes with multiple updates
- I read through the code and considered the security, stability and performance implications of the changes.
- I tested that the UI can be used with a keyboard only (tab order, keyboard controls).
- Includes tests for bugfixes and/or features added.
- Includes documentation
- Includes fixtures that create objects required for manual testing.
| mutationFn: revertLibraryChanges, | ||
| onSettled: (_data, _error, libraryId) => { | ||
| queryClient.invalidateQueries({ queryKey: libraryAuthoringQueryKeys.contentLibrary(libraryId) }); | ||
| queryClient.invalidateQueries({ queryKey: ['content_search'] }); |
There was a problem hiding this comment.
Q: is it really necessary to invalidate all cached data related to content search?
Nit: it would be nice to convert this to the query key pattern like we're doing above for library authoring query keys.
There was a problem hiding this comment.
Q: is it really necessary to invalidate all cached data related to content search?
@bradenmacdonald Actually, I only want to invalidate content search queries related to this library. So updated the new PR: openedx#1214 to include this commit: openedx@9a3fc04
I tried changing content search queries to use query key pattern but it has a lot of dynamic variables and I could not think of a nice way to implement it.
Related PRs:
Issue: openedx#1200
Test instructions:
tutor mounts add <path>.tutor dev launch -IDiscard changesbutton in sidebar.