Skip to content
Merged
Show file tree
Hide file tree
Changes from 11 commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
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
19 changes: 19 additions & 0 deletions README.rst
Original file line number Diff line number Diff line change
Expand Up @@ -250,6 +250,25 @@ Requirements
* ``contentstore.new_studio_mfe.use_new_export_page``: this feature flag will change the CMS to link to the new export page.
* ``contentstore.new_studio_mfe.use_new_import_page``: this feature flag will change the CMS to link to the new import page.

Feature: Tagging/Taxonomy Pages
================================

.. image:: ./docs/readme-images/feature-tagging-taxonomy-pages.png

Requirements
------------

* ``edx-platform`` Waffle flags:

* ``contentstore.new_studio_mfe.use_tagging_taxonomy_list_page``: this feature flag must be enabled.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

nit:

Suggested change
* ``contentstore.new_studio_mfe.use_tagging_taxonomy_list_page``: this feature flag must be enabled.
* ``new_studio_mfe.use_tagging_taxonomy_list_page``: this feature flag must be enabled.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This is correct, but all the other feature flags use contentstore.new_studio_mfe... so that was a small mistake on our part. Doesn't really matter though.


Configuration
-------------

In additional to the standard settings, the following local configuration items are required:

* ``ENABLE_TAGGING_TAXONOMY_PAGES``: must be enabled in order to actually present the new Tagging/Taxonomy pages.


Developing
**********
Expand Down
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
1 change: 1 addition & 0 deletions src/index.scss
Original file line number Diff line number Diff line change
Expand Up @@ -20,3 +20,4 @@
@import "export-page/CourseExportPage";
@import "import-page/CourseImportPage";
@import "files-and-uploads/table-components/GalleryCard";
@import "taxonomy/taxonomy-card/TaxonomyCard.scss";
79 changes: 0 additions & 79 deletions src/taxonomy/TaxonomyCard.jsx

This file was deleted.

2 changes: 1 addition & 1 deletion src/taxonomy/TaxonomyListPage.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import { StudioFooter } from '@edx/frontend-component-footer';
import Header from '../header';
import SubHeader from '../generic/sub-header/SubHeader';
import messages from './messages';
import TaxonomyCard from './TaxonomyCard';
import TaxonomyCard from './taxonomy-card/TaxonomyCard';
import { useTaxonomyListDataResponse, useIsTaxonomyListDataLoaded } from './api/hooks/selectors';

const TaxonomyListPage = ({ intl }) => {
Expand Down
14 changes: 10 additions & 4 deletions src/taxonomy/api/hooks/api.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,17 +4,23 @@ import { camelCaseObject, getConfig } from '@edx/frontend-platform';
import { getAuthenticatedHttpClient } from '@edx/frontend-platform/auth';

const getApiBaseUrl = () => getConfig().STUDIO_BASE_URL;
const getTaxonomyListApiUrl = new URL('api/content_tagging/v1/taxonomies/?enabled=true', getApiBaseUrl()).href;
const getTaxonomyListApiUrl = () => new URL('api/content_tagging/v1/taxonomies/?enabled=true', getApiBaseUrl()).href;
export const getExportTaxonomyApiUrl = (pk, format) => new URL(
`api/content_tagging/v1/taxonomies/${pk}/export/?output_format=${format}&download=1`,
getApiBaseUrl(),
).href;

/**
* @returns {import("../types.mjs").UseQueryResult}
*/
const useTaxonomyListData = () => (
export const useTaxonomyListData = () => (
useQuery({
queryKey: ['taxonomyList'],
queryFn: () => getAuthenticatedHttpClient().get(getTaxonomyListApiUrl)
queryFn: () => getAuthenticatedHttpClient().get(getTaxonomyListApiUrl())
.then(camelCaseObject),
})
);

export default useTaxonomyListData;
export const exportTaxonomy = (pk, format) => {
window.location.href = getExportTaxonomyApiUrl(pk, format);
};
37 changes: 34 additions & 3 deletions src/taxonomy/api/hooks/api.test.js
Original file line number Diff line number Diff line change
@@ -1,15 +1,19 @@
import { useQuery } from '@tanstack/react-query';
import useTaxonomyListData from './api';
import { useTaxonomyListData, exportTaxonomy } from './api';

const mockHttpClient = {
get: jest.fn(),
};

jest.mock('@tanstack/react-query', () => ({
useQuery: jest.fn(),
}));

jest.mock('@edx/frontend-platform/auth', () => ({
getAuthenticatedHttpClient: jest.fn(),
getAuthenticatedHttpClient: jest.fn(() => mockHttpClient),
}));

describe('taxonomy API: useTaxonomyListData', () => {
describe('useTaxonomyListData', () => {
afterEach(() => {
jest.clearAllMocks();
});
Expand All @@ -23,3 +27,30 @@ describe('taxonomy API: useTaxonomyListData', () => {
});
});
});

describe('exportTaxonomy', () => {
const { location } = window;

beforeAll(() => {
delete window.location;
window.location = {
href: '',
};
});

afterAll(() => {
window.location = location;
});

it('should set window.location.href correctly', () => {
const pk = 1;
const format = 'json';

exportTaxonomy(pk, format);

expect(window.location.href).toEqual(
'http://localhost:18010/api/content_tagging/'
+ 'v1/taxonomies/1/export/?output_format=json&download=1',
);
});
});
9 changes: 8 additions & 1 deletion src/taxonomy/api/hooks/selectors.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
// @ts-check
import useTaxonomyListData from './api';
import {
useTaxonomyListData,
exportTaxonomy,
} from './api';

/**
* @returns {import("../types.mjs").TaxonomyListData | undefined}
Expand All @@ -18,3 +21,7 @@ export const useTaxonomyListDataResponse = () => {
export const useIsTaxonomyListDataLoaded = () => (
useTaxonomyListData().status === 'success'
);

export const callExportTaxonomy = (pk, format) => (
exportTaxonomy(pk, format)
);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I don't think selectors is the correct place for this, although I think this might be a preexisting problem with this module.

I think it's important to keep this ADR in mind: https://github.com/openedx/frontend-template-application/blob/master/docs/decisions/0002-feature-based-application-organization.rst

None of the functions here are hooks or selectors in the commonly understood context of those terms.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@xitij2000 I updated the UI components to match with the ADR here: b09b86a

About the data layer, we have try to use useQuery to get data. On df8432d I tried to match with the structure on the ADR, @xitij2000 could you check it?

CC @bradenmacdonald

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Given that this is using useQuery instead of redux I'm not sure how things would map. However, I think it's best to avoid confusion with exist terms like hooks, thunks, selectors, if you are not using those since someone else working on this might have difficulty navigating this codebase in that case.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Thanks! I understand. I deleted thunks.js and I move hooks.jsx to api/apiHooks.jsx (to avoid use hooks or any other exist term). Also I added a docstring in api/apiHooks.jsx to explain all of this.

19 changes: 16 additions & 3 deletions src/taxonomy/api/hooks/selectors.test.js
Original file line number Diff line number Diff line change
@@ -1,9 +1,14 @@
import { useTaxonomyListDataResponse, useIsTaxonomyListDataLoaded } from './selectors';
import useTaxonomyListData from './api';
import {
useTaxonomyListDataResponse,
useIsTaxonomyListDataLoaded,
callExportTaxonomy,
} from './selectors';
import { useTaxonomyListData, exportTaxonomy } from './api';

jest.mock('./api', () => ({
__esModule: true,
default: jest.fn(),
useTaxonomyListData: jest.fn(),
exportTaxonomy: jest.fn(),
}));

describe('useTaxonomyListDataResponse', () => {
Expand Down Expand Up @@ -41,3 +46,11 @@ describe('useIsTaxonomyListDataLoaded', () => {
expect(result).toBe(false);
});
});

describe('callExportTaxonomy', () => {
it('should trigger exportTaxonomy', () => {
callExportTaxonomy(1, 'csv');

expect(exportTaxonomy).toHaveBeenCalled();
});
});
40 changes: 40 additions & 0 deletions src/taxonomy/messages.js
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,46 @@ const messages = defineMessages({
id: 'course-authoring.taxonomy-list.spinner.loading',
defaultMessage: 'Loading',
},
systemTaxonomyPopoverTitle: {
id: 'course-authoring.taxonomy-list.popover.system-defined.title',
defaultMessage: 'System taxonomy',
},
systemTaxonomyPopoverBody: {
id: 'course-authoring.taxonomy-list.popover.system-defined.body',
defaultMessage: 'This is a system-level taxonomy and is enabled by default.',
},
taxonomyCardExportMenu: {
id: 'course-authoring.taxonomy-list.menu.export.label',
defaultMessage: 'Export',
},
taxonomyMenuAlt: {
id: 'course-authoring.taxonomy-list.menu.alt',
defaultMessage: '{name} menu',
},
exportModalTitle: {
id: 'course-authoring.taxonomy-list.modal.export.title',
defaultMessage: 'Select format to export',
},
exportModalBodyDescription: {
id: 'course-authoring.taxonomy-list.modal.export.body',
defaultMessage: 'Select the file format in which you would like the taxonomy to be exported:',
},
exportModalSubmitButtonLabel: {
id: 'course-authoring.taxonomy-list.modal.export.submit.label',
defaultMessage: 'Export',
},
taxonomyCSVFormat: {
id: 'course-authoring.taxonomy-list.csv-format',
defaultMessage: 'CSV file',
},
taxonomyJSONFormat: {
id: 'course-authoring.taxonomy-list.json-format',
defaultMessage: 'JSON file',
},
taxonomyModalsCancelLabel: {
id: 'course-authoring.taxonomy-list.modal.cancel',
defaultMessage: 'Cancel',
},
});

export default messages;
86 changes: 86 additions & 0 deletions src/taxonomy/modals/ExportModal.jsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
import React, { useState } from 'react';
import {
ActionRow,
Button,
Form,
ModalDialog,
} from '@edx/paragon';
import PropTypes from 'prop-types';
import { injectIntl, intlShape } from '@edx/frontend-platform/i18n';
import messages from '../messages';
import { callExportTaxonomy } from '../api/hooks/selectors';

const ExportModal = ({
taxonomyId,
isOpen,
onClose,
intl,
}) => {
const [outputFormat, setOutputFormat] = useState('csv');

const onClickExport = () => {
onClose();
callExportTaxonomy(taxonomyId, outputFormat);
};

return (
<ModalDialog
title={intl.formatMessage(messages.exportModalTitle)}
isOpen={isOpen}
onClose={onClose}
size="lg"
hasCloseButton
isFullscreenOnMobile
>
<ModalDialog.Header>
<ModalDialog.Title>
{intl.formatMessage(messages.exportModalTitle)}
</ModalDialog.Title>
</ModalDialog.Header>
<ModalDialog.Body className="pb-5 mt-2">
<Form.Group>
<Form.Label>
{intl.formatMessage(messages.exportModalBodyDescription)}
</Form.Label>
<Form.RadioSet
name="export-format"
value={outputFormat}
onChange={(e) => setOutputFormat(e.target.value)}
>
<Form.Radio
key={`export-csv-format-${taxonomyId}`}
value="csv"
>
{intl.formatMessage(messages.taxonomyCSVFormat)}
</Form.Radio>
<Form.Radio
key={`export-json-format-${taxonomyId}`}
value="json"
>
{intl.formatMessage(messages.taxonomyJSONFormat)}
</Form.Radio>
</Form.RadioSet>
</Form.Group>
</ModalDialog.Body>
<ModalDialog.Footer>
<ActionRow>
<ModalDialog.CloseButton variant="tertiary">
{intl.formatMessage(messages.taxonomyModalsCancelLabel)}
</ModalDialog.CloseButton>
<Button variant="primary" onClick={onClickExport}>
{intl.formatMessage(messages.exportModalSubmitButtonLabel)}
</Button>
</ActionRow>
</ModalDialog.Footer>
</ModalDialog>
);
};

ExportModal.propTypes = {
taxonomyId: PropTypes.number.isRequired,
isOpen: PropTypes.bool.isRequired,
onClose: PropTypes.func.isRequired,
intl: intlShape.isRequired,
};

export default injectIntl(ExportModal);
Loading