Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: tag sections, subsections, and the whole course (FC-0053) #879

Merged
11 changes: 10 additions & 1 deletion src/content-tags-drawer/ContentTagsDrawer.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,15 @@
} = useContentTaxonomyTagsData(contentId);
const { data: taxonomyListData, isSuccess: isTaxonomyListLoaded } = useTaxonomyList(org);

let contentName = '';
if (isContentDataLoaded) {
if ('displayName' in contentData) {
contentName = contentData.displayName;
} else {
contentName = contentData.courseDisplayNameWithDefault;

Check warning on line 84 in src/content-tags-drawer/ContentTagsDrawer.jsx

View check run for this annotation

Codecov / codecov/patch

src/content-tags-drawer/ContentTagsDrawer.jsx#L83-L84

Added lines #L83 - L84 were not covered by tests
}
}

let onCloseDrawer = onClose;
if (onCloseDrawer === undefined) {
onCloseDrawer = () => {
Expand Down Expand Up @@ -129,7 +138,7 @@
<CloseButton onClick={() => onCloseDrawer()} data-testid="drawer-close-button" />
<span>{intl.formatMessage(messages.headerSubtitle)}</span>
{ isContentDataLoaded
? <h3>{ contentData.displayName }</h3>
? <h3>{ contentName }</h3>
: (
<div className="d-flex justify-content-center align-items-center flex-column">
<Spinner
Expand Down
12 changes: 9 additions & 3 deletions src/content-tags-drawer/data/api.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
};
export const getContentTaxonomyTagsApiUrl = (contentId) => new URL(`api/content_tagging/v1/object_tags/${contentId}/`, getApiBaseUrl()).href;
export const getXBlockContentDataApiURL = (contentId) => new URL(`/xblock/outline/${contentId}`, getApiBaseUrl()).href;
export const getCourseContentDataApiURL = (contentId) => new URL(`/api/contentstore/v1/course_settings/${contentId}`, getApiBaseUrl()).href;
export const getLibraryContentDataApiUrl = (contentId) => new URL(`/api/libraries/v2/blocks/${contentId}/`, getApiBaseUrl()).href;
export const getContentTaxonomyTagsCountApiUrl = (contentId) => new URL(`api/content_tagging/v1/object_tag_counts/${contentId}/?count_implicit`, getApiBaseUrl()).href;

Expand Down Expand Up @@ -74,9 +75,14 @@
* @returns {Promise<import("./types.mjs").ContentData>}
*/
export async function getContentData(contentId) {
const url = contentId.startsWith('lb:')
? getLibraryContentDataApiUrl(contentId)
: getXBlockContentDataApiURL(contentId);
let url;
if (contentId.startsWith('lb:')) {
url = getLibraryContentDataApiUrl(contentId);
} else if (contentId.startsWith('course-v1:')) {
url = getCourseContentDataApiURL(contentId);

Check warning on line 82 in src/content-tags-drawer/data/api.js

View check run for this annotation

Codecov / codecov/patch

src/content-tags-drawer/data/api.js#L82

Added line #L82 was not covered by tests
} else {
url = getXBlockContentDataApiURL(contentId);
}
const { data } = await getAuthenticatedHttpClient().get(url);
return camelCaseObject(data);
}
Expand Down
23 changes: 9 additions & 14 deletions src/content-tags-drawer/data/apiHooks.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ import {
getContentTaxonomyTagsData,
getContentData,
updateContentTaxonomyTags,
getContentTaxonomyTagsCount,
} from './api';

/** @typedef {import("../../taxonomy/tag-list/data/types.mjs").TagListData} TagListData */
Expand Down Expand Up @@ -106,17 +105,6 @@ export const useContentTaxonomyTagsData = (contentId) => (
})
);

/**
* Build the query to get the count og taxonomy tags applied to the content object
* @param {string} contentId The ID of the content object to fetch the count of the applied tags for
*/
export const useContentTaxonomyTagsCount = (contentId) => (
useQuery({
queryKey: ['contentTaxonomyTagsCount', contentId],
queryFn: () => getContentTaxonomyTagsCount(contentId),
})
);

/**
* Builds the query to get meta data about the content object
* @param {string} contentId The id of the content object (unit/component)
Expand Down Expand Up @@ -150,8 +138,15 @@ export const useContentTaxonomyTagsUpdater = (contentId, taxonomyId) => {
onSettled: /* istanbul ignore next */ () => {
queryClient.invalidateQueries({ queryKey: ['contentTaxonomyTags', contentId] });
/// Invalidate query with pattern on course outline
queryClient.invalidateQueries({ queryKey: ['unitTagsCount'] });
queryClient.invalidateQueries({ queryKey: ['contentTaxonomyTagsCount', contentId] });
let contentPattern;
if (contentId.includes('course-v1')) {
contentPattern = contentId;
} else {
contentPattern = contentId.replace(/\+type@.*$/, '*');
}
queryClient.invalidateQueries({ queryKey: ['contentTagsCount', contentPattern] });
// FixMe: remove code below
// queryClient.invalidateQueries({ queryKey: ['contentTaxonomyTagsCount', contentId] });
Copy link
Contributor

Choose a reason for hiding this comment

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

I guess this is left behind by mistake?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure! Sorry for that @xitij2000!
Fixed: 643cc1e

},
});
};
19 changes: 0 additions & 19 deletions src/content-tags-drawer/data/apiHooks.test.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import {
useContentTaxonomyTagsData,
useContentData,
useContentTaxonomyTagsUpdater,
useContentTaxonomyTagsCount,
} from './apiHooks';

import { updateContentTaxonomyTags } from './api';
Expand Down Expand Up @@ -135,24 +134,6 @@ describe('useContentTaxonomyTagsData', () => {
});
});

describe('useContentTaxonomyTagsCount', () => {
it('should return success response', () => {
useQuery.mockReturnValueOnce({ isSuccess: true, data: 'data' });
const contentId = '123';
const result = useContentTaxonomyTagsCount(contentId);

expect(result).toEqual({ isSuccess: true, data: 'data' });
});

it('should return failure response', () => {
useQuery.mockReturnValueOnce({ isSuccess: false });
const contentId = '123';
const result = useContentTaxonomyTagsCount(contentId);

expect(result).toEqual({ isSuccess: false });
});
});

describe('useContentData', () => {
it('should return success response', () => {
useQuery.mockReturnValueOnce({ isSuccess: true, data: 'data' });
Expand Down
11 changes: 10 additions & 1 deletion src/content-tags-drawer/data/types.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@
*/

/**
* @typedef {Object} ContentData
* @typedef {Object} XBlockData
* @property {string} id
* @property {string} displayName
* @property {string} category
Expand Down Expand Up @@ -58,3 +58,12 @@
* @property {boolean} staffOnlyMessage
* @property {boolean} hasPartitionGroupComponents
*/

/**
* @typedef {Object} CourseData
* @property {string} courseDisplayNameWithDefault
*/

/**
* @typedef {XBlockData | CourseData} ContentData
*/
Original file line number Diff line number Diff line change
Expand Up @@ -4,18 +4,18 @@ import { Stack } from '@openedx/paragon';
import { useParams } from 'react-router-dom';
import { useIntl } from '@edx/frontend-platform/i18n';

import { useContentTagsCount } from '../../generic/data/apiHooks';
import messages from '../messages';
import { useContentTaxonomyTagsCount } from '../data/apiHooks';
import TagCount from '../../generic/tag-count';

const TagsSidebarHeader = () => {
const intl = useIntl();
const contentId = useParams().blockId;

const {
data: contentTaxonomyTagsCount,
isSuccess: isContentTaxonomyTagsCountLoaded,
} = useContentTaxonomyTagsCount(contentId || '');
data: contentTagsCount,
isSuccess: isContentTagsCountLoaded,
} = useContentTagsCount(contentId || '');

return (
<Stack
Expand All @@ -25,8 +25,8 @@ const TagsSidebarHeader = () => {
<h3 className="course-unit-sidebar-header-title m-0">
{intl.formatMessage(messages.tagsSidebarTitle)}
</h3>
{ isContentTaxonomyTagsCountLoaded
&& <TagCount count={contentTaxonomyTagsCount} />}
{ isContentTagsCountLoaded
&& <TagCount count={contentTagsCount} />}
</Stack>
);
};
Expand Down
Original file line number Diff line number Diff line change
@@ -1,36 +1,39 @@
import React from 'react';
import { render, screen } from '@testing-library/react';
import { IntlProvider } from '@edx/frontend-platform/i18n';
import { QueryClient, QueryClientProvider } from '@tanstack/react-query';
import TagsSidebarHeader from './TagsSidebarHeader';
import { useContentTaxonomyTagsCount } from '../data/apiHooks';

jest.mock('../data/apiHooks', () => ({
useContentTaxonomyTagsCount: jest.fn(() => ({
isSuccess: false,
data: 17,
})),
const mockGetTagsCount = jest.fn();

jest.mock('../../generic/data/api', () => ({
...jest.requireActual('../../generic/data/api'),
getTagsCount: () => mockGetTagsCount(),
}));

jest.mock('react-router-dom', () => ({
...jest.requireActual('react-router-dom'),
useParams: () => ({ blockId: '123' }),
}));

const queryClient = new QueryClient();

const RootWrapper = () => (
<IntlProvider locale="en" messages={{}}>
<TagsSidebarHeader />
<QueryClientProvider client={queryClient}>
<TagsSidebarHeader />
</QueryClientProvider>
</IntlProvider>
);

describe('<TagsSidebarHeader>', () => {
it('should not render count on loading', () => {
it('should render count only after query is complete', async () => {
let resolvePromise;
mockGetTagsCount.mockReturnValueOnce(new Promise((resolve) => { resolvePromise = resolve; }));
render(<RootWrapper />);
expect(screen.getByRole('heading', { name: /unit tags/i })).toBeInTheDocument();
expect(screen.queryByText('17')).not.toBeInTheDocument();
});

it('should render count after query is complete', () => {
useContentTaxonomyTagsCount.mockReturnValue({
isSuccess: true,
data: 17,
});
render(<RootWrapper />);
expect(screen.getByRole('heading', { name: /unit tags/i })).toBeInTheDocument();
expect(screen.getByText('17')).toBeInTheDocument();
resolvePromise({ 123: 17 });
expect(await screen.findByText('17')).toBeInTheDocument();
});
});
31 changes: 3 additions & 28 deletions src/course-outline/CourseOutline.jsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { useState, useEffect, useMemo } from 'react';
// @ts-check
import React, { useState, useEffect } from 'react';
import PropTypes from 'prop-types';
import { useIntl } from '@edx/frontend-platform/i18n';
import {
Expand Down Expand Up @@ -52,7 +53,6 @@ import {
} from './drag-helper/utils';
import { useCourseOutline } from './hooks';
import messages from './messages';
import useUnitTagsCount from './data/apiHooks';

const CourseOutline = ({ courseId }) => {
const intl = useIntl();
Expand Down Expand Up @@ -113,7 +113,6 @@ const CourseOutline = ({ courseId }) => {
mfeProctoredExamSettingsUrl,
handleDismissNotification,
advanceSettingsUrl,
prevContainerInfo,
handleSectionDragAndDrop,
handleSubsectionDragAndDrop,
handleUnitDragAndDrop,
Expand All @@ -133,27 +132,6 @@ const CourseOutline = ({ courseId }) => {
const { category } = useSelector(getCurrentItem);
const deleteCategory = COURSE_BLOCK_NAMES[category]?.name.toLowerCase();

const unitsIdPattern = useMemo(() => {
let pattern = '';
sections.forEach((section) => {
section.childInfo.children.forEach((subsection) => {
subsection.childInfo.children.forEach((unit) => {
if (pattern !== '') {
pattern += `,${unit.id}`;
} else {
pattern += unit.id;
}
});
});
});
return pattern;
}, [sections]);

const {
data: unitsTagCounts,
isSuccess: isUnitsTagCountsLoaded,
} = useUnitTagsCount(unitsIdPattern);

/**
* Move section to new index
* @param {any} currentIndex
Expand Down Expand Up @@ -268,7 +246,6 @@ const CourseOutline = ({ courseId }) => {
) : null}
</TransitionReplace>
<SubHeader
className="mt-5"
title={intl.formatMessage(messages.headingTitle)}
subtitle={intl.formatMessage(messages.headingSubtitle)}
headerActions={(
Expand Down Expand Up @@ -307,7 +284,6 @@ const CourseOutline = ({ courseId }) => {
items={sections}
setSections={setSections}
restoreSectionList={restoreSectionList}
prevContainerInfo={prevContainerInfo}
handleSectionDragAndDrop={handleSectionDragAndDrop}
handleSubsectionDragAndDrop={handleSubsectionDragAndDrop}
handleUnitDragAndDrop={handleUnitDragAndDrop}
Expand All @@ -319,7 +295,6 @@ const CourseOutline = ({ courseId }) => {
>
{sections.map((section, sectionIndex) => (
<SectionCard
id={section.id}
key={section.id}
section={section}
index={sectionIndex}
Expand Down Expand Up @@ -398,7 +373,6 @@ const CourseOutline = ({ courseId }) => {
onOrderChange={updateUnitOrderByIndex}
onCopyToClipboardClick={handleCopyToClipboardClick}
discussionsSettings={discussionsSettings}
tagsCount={isUnitsTagCountsLoaded ? unitsTagCounts[unit.id] : 0}
/>
))}
</SortableContext>
Expand Down Expand Up @@ -482,6 +456,7 @@ const CourseOutline = ({ courseId }) => {
variant="danger"
icon={WarningIcon}
title={intl.formatMessage(messages.alertErrorTitle)}
description=""
aria-hidden="true"
/>
)}
Expand Down
17 changes: 11 additions & 6 deletions src/course-outline/CourseOutline.test.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import { AppProvider } from '@edx/frontend-platform/react';
import { initializeMockApp } from '@edx/frontend-platform';
import MockAdapter from 'axios-mock-adapter';
import { getAuthenticatedHttpClient } from '@edx/frontend-platform/auth';
import { QueryClient, QueryClientProvider } from '@tanstack/react-query';
import { cloneDeep } from 'lodash';
import { closestCorners } from '@dnd-kit/core';

Expand Down Expand Up @@ -85,11 +86,13 @@ jest.mock('@edx/frontend-platform/i18n', () => ({
}),
}));

jest.mock('./data/apiHooks', () => () => ({
data: {},
isSuccess: true,
jest.mock('./data/api', () => ({
...jest.requireActual('./data/api'),
getTagsCount: () => jest.fn().mockResolvedValue({}),
}));

const queryClient = new QueryClient();

jest.mock('@dnd-kit/core', () => ({
...jest.requireActual('@dnd-kit/core'),
// Since jsdom (used by jest) does not support getBoundingClientRect function
Expand All @@ -104,9 +107,11 @@ const sleep = (ms) => new Promise((resolve) => setTimeout(resolve, ms));

const RootWrapper = () => (
<AppProvider store={store}>
<IntlProvider locale="en">
<CourseOutline courseId={courseId} />
</IntlProvider>
<QueryClientProvider client={queryClient}>
<IntlProvider locale="en">
<CourseOutline courseId={courseId} />
</IntlProvider>
</QueryClientProvider>
</AppProvider>
);

Expand Down
1 change: 0 additions & 1 deletion src/course-outline/__mocks__/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,4 +4,3 @@ export { default as courseBestPracticesMock } from './courseBestPractices';
export { default as courseLaunchMock } from './courseLaunch';
export { default as courseSectionMock } from './courseSection';
export { default as courseSubsectionMock } from './courseSubsection';
export { default as contentTagsCountMock } from './contentTagsCount';
Loading
Loading