Skip to content
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 43 additions & 5 deletions src/content-tags-drawer/ContentTagsDrawer.test.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ const {
languageWithoutTagsId,
largeTagsId,
emptyTagsId,
containerTagsId,
} = mockContentTaxonomyTagsData;

jest.mock('react-router-dom', () => ({
Expand All @@ -46,14 +47,15 @@ jest.mock('../library-authoring/common/context/SidebarContext', () => ({
useSidebarContext: () => ({ sidebarAction: mockSidebarAction() }),
}));

const renderDrawer = (contentId, drawerParams = {}) => (
render(
const renderDrawer = (contentId, drawerParams = {}, renderPath = path, containerId = '') => {
const params = { contentId, containerId };
return render(
<ContentTagsDrawerSheetContext.Provider value={drawerParams}>
<ContentTagsDrawer {...drawerParams} />
</ContentTagsDrawerSheetContext.Provider>,
{ path, params: { contentId } },
)
);
{ path: renderPath, params },
);
};

describe('<ContentTagsDrawer />', () => {
beforeEach(async () => {
Expand Down Expand Up @@ -692,6 +694,42 @@ describe('<ContentTagsDrawer />', () => {
await waitFor(() => expect(axiosMock.history.put[0].url).toEqual(url));
});

[
'lct:org:lib:unit:1',
'lib-collection:org:lib:1',
'lb:org:lib:html:1',
].forEach((containerId) => {
it(`should invalidate children query when update child tag when containerId is ${containerId}`, async () => {
const newPath = '/container/:containerId/';
const { axiosMock, queryClient } = initializeMocks();
const mockInvalidateQueries = jest.spyOn(queryClient, 'invalidateQueries');
const url = getContentTaxonomyTagsApiUrl(containerTagsId);
axiosMock.onPut(url).reply(200);
renderDrawer(containerTagsId, { id: containerTagsId }, newPath, containerId);
expect(await screen.findByText('Taxonomy 1')).toBeInTheDocument();
const editTagsButton = screen.getByRole('button', {
name: /edit tags/i,
});
fireEvent.click(editTagsButton);

const saveButton = screen.getByRole('button', {
name: /save/i,
});
fireEvent.click(saveButton);

await waitFor(() => expect(axiosMock.history.put[0].url).toEqual(url));
expect(mockInvalidateQueries).toHaveBeenCalledTimes(5);
expect(mockInvalidateQueries).toHaveBeenNthCalledWith(5, [
'contentLibrary',
'lib:org:lib',
'content',
'container',
containerId,
'children',
]);
});
});

it('should taxonomies must be ordered', async () => {
renderDrawer(largeTagsId);
expect(await screen.findByText('Taxonomy 1')).toBeInTheDocument();
Expand Down
2 changes: 1 addition & 1 deletion src/content-tags-drawer/data/api.mocks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -205,7 +205,7 @@ mockContentTaxonomyTagsData.emptyTagsId = 'block-v1:EmptyTagsOrg+STC1+2023_1+typ
mockContentTaxonomyTagsData.emptyTags = {
taxonomies: [],
};
mockContentTaxonomyTagsData.containerTagsId = 'lct:org:lib:unit:container_tags';
mockContentTaxonomyTagsData.containerTagsId = 'lct:StagedTagsOrg:lib:unit:container_tags';
mockContentTaxonomyTagsData.applyMock = () => jest.spyOn(api, 'getContentTaxonomyTagsData').mockImplementation(mockContentTaxonomyTagsData);

/**
Expand Down
13 changes: 7 additions & 6 deletions src/content-tags-drawer/data/apiHooks.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ export const useContentTaxonomyTagsData = (contentId) => (

/**
* Builds the query to get meta data about the content object
* @param {string} contentId The id of the content object (unit/component)
* @param {string} contentId The id of the content object
* @param {boolean} enabled Flag to enable/disable the query
*/
export const useContentData = (contentId, enabled) => (
Expand All @@ -130,7 +130,7 @@ export const useContentData = (contentId, enabled) => (
export const useContentTaxonomyTagsUpdater = (contentId) => {
const queryClient = useQueryClient();
const unitIframe = window.frames['xblock-iframe'];
const { unitId } = useParams();
const { containerId } = useParams();

return useMutation({
/**
Expand All @@ -143,7 +143,7 @@ export const useContentTaxonomyTagsUpdater = (contentId) => {
* >}
*/
mutationFn: ({ tagsData }) => updateContentTaxonomyTags(contentId, tagsData),
onSettled: /* istanbul ignore next */ () => {
onSettled: () => {
queryClient.invalidateQueries({ queryKey: ['contentTaxonomyTags', contentId] });
/// Invalidate query with pattern on course outline
let contentPattern;
Expand All @@ -160,9 +160,10 @@ export const useContentTaxonomyTagsUpdater = (contentId) => {
queryClient.invalidateQueries(xblockQueryKeys.componentMetadata(contentId));
// Invalidate content search to update tags count
queryClient.invalidateQueries(['content_search'], { predicate: (query) => libraryQueryPredicate(query, libraryId) });
// If the tags for a compoent were edited from Unit page, invalidate children query to fetch count again.
if (unitId) {
queryClient.invalidateQueries(libraryAuthoringQueryKeys.containerChildren(unitId));
// If the tags for an item were edited from a container page (Unit, Subsection, Section),
// invalidate children query to fetch count again.
if (containerId) {
queryClient.invalidateQueries(libraryAuthoringQueryKeys.containerChildren(containerId));
}
}
},
Expand Down
2 changes: 1 addition & 1 deletion src/generic/tag-count/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ type TagCountProps = {
};

// eslint-disable-next-line react/prop-types
const TagCount: React.FC<TagCountProps> = ({ count, onClick, size }) => {
const TagCount: React.FC<TagCountProps> = ({ count, onClick, size }: TagCountProps) => {
const renderContent = () => (
<Stack direction="horizontal" gap={1}>
<Icon size={size} src={Tag} />
Expand Down
5 changes: 4 additions & 1 deletion src/library-authoring/data/apiHooks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -703,7 +703,10 @@ export const useContainerChildren = (containerId?: string, published: boolean =
enabled: !!containerId,
queryKey: libraryAuthoringQueryKeys.containerChildren(containerId!),
queryFn: () => api.getLibraryContainerChildren(containerId!, published),
structuralSharing: (oldData: api.LibraryBlockMetadata[], newData: api.LibraryBlockMetadata[]) => {
structuralSharing: (
oldData: api.LibraryBlockMetadata[] | api.Container[],
newData: api.LibraryBlockMetadata[] | api.Container[],
) => {
// This just sets `isNew` flag to new children components
if (oldData) {
const oldDataIds = oldData.map((obj) => obj.id);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,8 @@ import { ToastContext } from '../../generic/toast-context';
import TagCount from '../../generic/tag-count';
import { ContainerMenu } from '../components/ContainerCard';
import { useLibraryRoutes } from '../routes';
import { useSidebarContext } from '../common/context/SidebarContext';
import { SidebarActions, useSidebarContext } from '../common/context/SidebarContext';
import { useRunOnNextRender } from '../../utils';

interface LibraryContainerChildrenProps {
containerKey: string;
Expand All @@ -45,6 +46,8 @@ const ContainerRow = ({ containerKey, container, readOnly }: ContainerRowProps)
const { showToast } = useContext(ToastContext);
const updateMutation = useUpdateContainer(container.originalId, containerKey);
const { showOnlyPublished } = useLibraryContext();
const { navigateTo } = useLibraryRoutes();
const { setSidebarAction } = useSidebarContext();

const handleSaveDisplayName = async (newDisplayName: string) => {
try {
Expand All @@ -57,6 +60,18 @@ const ContainerRow = ({ containerKey, container, readOnly }: ContainerRowProps)
}
};

/* istanbul ignore next */
const scheduleJumpToTags = useRunOnNextRender(() => {
// TODO: Ugly hack to make sure sidebar shows manage tags section
// This needs to run after all changes to url takes place to avoid conflicts.
setTimeout(() => setSidebarAction(SidebarActions.JumpToManageTags), 250);
});
Comment on lines +64 to +68
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wow, I wish there were a better way to do this. I see we're already using it in a few other places.

What is the reason we can't just do something like

navigateTo({ selectedItemId: container.originalId, sidebarAction: SidebarActions.JumpToManageTags });

?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am working on this in open-craft#91


const jumpToManageTags = () => {
navigateTo({ selectedItemId: container.originalId });
scheduleJumpToTags();
};

return (
<>
{/* eslint-disable-next-line jsx-a11y/click-events-have-key-events, jsx-a11y/no-static-element-interactions */}
Expand Down Expand Up @@ -90,7 +105,11 @@ const ContainerRow = ({ containerKey, container, readOnly }: ContainerRowProps)
</Stack>
</Badge>
)}
<TagCount size="sm" count={container.tagsCount} />
<TagCount
size="sm"
count={container.tagsCount}
onClick={readOnly ? undefined : jumpToManageTags}
/>
{!readOnly && (
<ContainerMenu
containerKey={container.originalId}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -389,5 +389,25 @@ describe('<LibrarySectionPage / LibrarySubsectionPage />', () => {
expect((await screen.findAllByText(new RegExp(`${childType} block 0`, 'i')))[0]).toBeInTheDocument();
expect(await screen.findByRole('button', { name: new RegExp(`${childType} Info`, 'i') })).toBeInTheDocument();
});

it(`should open manage tags on click tag count in ${cType} page`, async () => {
const cId = cType === ContainerType.Section
? mockGetContainerMetadata.sectionId
: mockGetContainerMetadata.subsectionId;
renderLibrarySectionPage(cId, undefined, cType);
// check all children components are rendered.
expect((await screen.findAllByText(`${childType} block 0`))[0]).toBeInTheDocument();
expect((await screen.findAllByText(`${childType} block 1`))[0]).toBeInTheDocument();
expect((await screen.findAllByText(`${childType} block 2`))[0]).toBeInTheDocument();

const tagCountButton = screen.getAllByRole('button', { name: '0' })[0];
fireEvent.click(tagCountButton);

expect(await screen.findByTestId('library-sidebar')).toBeInTheDocument();
await waitFor(
() => expect(screen.getByRole('tab', { name: /manage/i })).toHaveClass('active'),
{ timeout: 300 },
);
});
});
});