feat: add import taxonomy feature [FC-0036] #675

rpenido · 2023-11-08T20:45:35Z

Description

This PR adds a new button in the Taxonomy List to allow users to create new taxonomies by importing a CSV/JSON file.

It also adds a new menu item in the Taxonomy Card menu and the Taxonomy Details menu to allow users to import tags to existing Taxonomies by importing a CSV/JSON file. This overwrites the current tags.

Supporting Information

Closes:
- [Tagging] Import a Taxonomy ("Bare Bones") modular-learning#115
- [Tagging] "Bare Bones" Update of Existing Taxonomy by Uploading modular-learning#140
Depends on:
- feat: add import taxonomy endpoint edx-platform#33663
- fix: overwrite previous tags on import [FC-0036] edx-platform#33752

Testing instructions

Import a new taxonomy

Have an import taxonomy template ready to be imported. You could get one from below:
- template.csv
- template.json
Go to the taxonomy list page (http://localhost:2001/taxonomies)
Click on the + Import button and select the desired file
You will be prompted to input JSON/CSV file, a Taxonomy Name and Description. The Taxonomy Name is required (not blank).
Canceling the prompt will abort the operation
If the taxonomy is imported successfully, you will see an alert: Taxonomy imported successfully.
Importing an invalid taxonomy file, you will see an alert: Import failed - see details in the browser console. The error info will be logged in the browser console.

Re-import tags to an existing taxonomy

Have an import taxonomy template ready to be imported. You could get one from below:
- template.csv
- template.json
Go to the taxonomy list page (http://localhost:2001/taxonomies)
Click on the Re-import menu from an existing taxonomy card
You will be prompted to select a file to be imported.
Canceling the prompt will abort the operation
If the taxonomy is imported successfully, you will see an alert: Taxonomy imported successfully.
Importing an invalid taxonomy file, you will see an alert: Import failed - see details in the browser console. The error info will be logged in the browser console.

Private-ref:

openedx-webhooks · 2023-11-08T20:45:40Z

Thanks for the pull request, @rpenido! Please note that it may take us up to several weeks or months to complete a review and merge your PR.

Feel free to add as much of the following information to the ticket as you can:

supporting documentation
Open edX discussion forum threads
timeline information ("this must be merged by XX date", and why that is)
partner information ("this is a course on edx.org")
any other information that can help Product understand the context for the PR

All technical communication about the code itself will be done via the GitHub pull request interface. As a reminder, our process documentation is here.

Please let us know once your PR is ready for our review and all tests are green.

codecov · 2023-11-08T20:53:06Z

Codecov Report

Attention: 1 lines in your changes are missing coverage. Please review.

Comparison is base (2205506) 89.61% compared to head (68e820d) 89.62%.

Files	Patch %	Lines
src/taxonomy/taxonomy-menu/TaxonomyMenu.jsx	97.05%	1 Missing ⚠️

Additional details and impacted files

@@           Coverage Diff           @@
##           master     #675   +/-   ##
=======================================
  Coverage   89.61%   89.62%           
=======================================
  Files         497      499    +2     
  Lines        8109     8085   -24     
  Branches     1708     1705    -3     
=======================================
- Hits         7267     7246   -21     
+ Misses        815      812    -3     
  Partials       27       27

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

rpenido · 2023-11-10T01:14:08Z

@rpenido I think you're missing some style changes from the original PR? I'm seeing this when I run this branch:

I changed the Dropdown component to make the tests run and broke the layout.
Fixed here: 4515f9f

pomegranited

This looks good @rpenido ! I've left a couple of minor comments which shouldn't be hard to address.

Could you also please update the PR description and un-draft this to get it ready for upstream/CC review?

👍 pending those changes ^

I tested this on my devstack while running the branches from the related PRs
I read through the code
I checked for accessibility issues by using my keyboard to navigate.
Includes documentation
User-facing strings are extracted for translation

src/taxonomy/import-tags/index.js

src/taxonomy/import-tags/data/api.test.js

jest.config.js

Co-authored-by: Jillian <jill@opencraft.com>

pomegranited · 2023-11-13T00:55:54Z

@bradenmacdonald This PR is also ready for upstream review -- who should we ask?

bradenmacdonald · 2023-11-13T20:22:38Z

src/taxonomy/import-tags/data/utils.js

+    * This function is a temporary "Barebones" implementation of the import
+    * functionality with `prompt` and `alert`. It is intended to be replaced
+    * with a component that shows a `ModalDialog` in the future.
+    * See: https://github.com/openedx/modular-learning/issues/116


^ Just flagging this in particular in case any reviewers are concerned about alert() and prompt() :) . This is just a temporary state during development to help get more PRs out faster and more iteratively, and will be replaced before any users ever see this (other than early beta testers).

bradenmacdonald · 2023-11-13T20:23:30Z

@jristau1984 another one - could you please line up a review for this PR too? (Or as mentioned, if you folks don't have much capacity for the near future, let me know and I can find a Core Contributor reviewer from Axim or OpenCraft.)

openedx-webhooks · 2023-11-14T13:56:05Z

I've created issue TNL-11201 in the private 2U Jira.

bradenmacdonald · 2023-11-14T18:38:32Z

@rpenido A couple of your PRs will need to be updated with master, including this one.

xitij2000

These are mostly nits. It's looking good overall.
I just need to test it once.

xitij2000 · 2023-12-27T05:13:38Z

src/taxonomy/TaxonomyListPage.test.jsx

+  it('calls the import taxonomy action when the import button is clicked', async () => {
    useIsTaxonomyListDataLoaded.mockReturnValue(true);
    useTaxonomyListDataResponse.mockReturnValue({
-      results: taxonomies,
+      results: [{
+        id: 1,
+        name: 'Taxonomy',
+        description: 'This is a description',
+      }],
    });
-    mockDeleteTaxonomy.mockImplementationOnce(async (params, callbacks) => {
-      callbacks.onSuccess();
+    await act(async () => {
+      const { getByTestId } = render(<RootWrapper />);
+      const importButton = getByTestId('taxonomy-import-button');
+      expect(importButton).toBeInTheDocument();
+      importButton.click();
+      expect(importTaxonomy).toHaveBeenCalled();
    });
-    const { getByTestId, getByLabelText } = render(<RootWrapper />);
-    fireEvent.click(getByTestId('test-delete-button'));
-    fireEvent.change(getByLabelText('Type DELETE to confirm'), { target: { value: 'DELETE' } });
-    fireEvent.click(getByTestId('delete-button'));
-
-    expect(mockDeleteTaxonomy).toBeCalledTimes(1);
-    expect(mockSetToastMessage).toBeCalledWith(`"${taxonomies[0].name}" deleted`);


I think this test can be skipped since it isn't doing anything. It's testing the presence of the import button, and then checking that clicking on the button fires the click handler. I think it should either be a deeper test of that UI or just be skipped.

This is only for test coverage. Is it ok to skip it? More about this in the comment at api.test.js

Hmm... I think that is a recurring issue. I've made a suggestion for the test, other than that I guess we should leave it for now.

xitij2000 · 2023-12-27T05:19:48Z

src/taxonomy/import-tags/data/api.test.js

+import MockAdapter from 'axios-mock-adapter';
+import { initializeMockApp } from '@edx/frontend-platform';
+import { getAuthenticatedHttpClient } from '@edx/frontend-platform/auth';
+
+import { tagImportMock, taxonomyImportMock } from '../__mocks__';
+
+import {
+  getTaxonomyImportNewApiUrl,
+  getTagsImportApiUrl,
+  importNewTaxonomy,
+  importTags,
+} from './api';
+
+let axiosMock;
+
+describe('import taxonomy api calls', () => {
+  beforeEach(() => {
+    initializeMockApp({
+      authenticatedUser: {
+        userId: 3,
+        username: 'abc123',
+        administrator: true,
+        roles: [],
+      },
+    });
+    axiosMock = new MockAdapter(getAuthenticatedHttpClient());
+  });
+
+  afterEach(() => {
+    jest.clearAllMocks();
+  });
+
+  it('should call import new taxonomy', async () => {
+    axiosMock.onPost(getTaxonomyImportNewApiUrl()).reply(201, taxonomyImportMock);
+    const result = await importNewTaxonomy('Taxonomy name', 'Taxonomy description');
+
+    expect(axiosMock.history.post[0].url).toEqual(getTaxonomyImportNewApiUrl());
+    expect(result).toEqual(taxonomyImportMock);
+  });
+
+  it('should call import tags', async () => {
+    axiosMock.onPut(getTagsImportApiUrl(1)).reply(200, tagImportMock);
+    const result = await importTags(1);
+
+    expect(axiosMock.history.put[0].url).toEqual(getTagsImportApiUrl(1));
+    expect(result).toEqual(tagImportMock);
+  });
+});


I think you can combine this with the UI test.

i.e. drive the UI and then check the API response.

I followed this pattern throughout the code. I would like your opinion on my rationale before changing this (for this PR and all other frontend features that I did).
With the ADR0002 in mind, I tried to isolate the features in the tests to make it "self-contained".

In this particular case, the UI is in the taxonomy-menu feature and the actual import code is in the taxonomy-import.
So, I wrote all the tests with these abstractions in mind:

I test if the UI calls a function (or shows a dialog) that will start a flow from another feature and handle its results (like calling the function to show a toast)

I test that this feature function/dialog works and calls the appropriate hooks/apis

If there is some API hook in the feature, I wrote a unit test to check if this hook makes the correct API function call and side effects (i.e., invalidate other queries)

And finally, I check if the API function calls make the correct HTTP request with the correct parameters

I saw comments in other PRs wanting to make these tests more "combined" or "user-oriented", using the UI and checking the Axios calls. I understand that, but I found it difficult to implement without having to:

Break the "feature barrier"

Duplicating these tests if we reuse some api/hook/feature in more than one place

Making the tests too complex (the same tests check the UI components, the HTTP calls and responses, react-query invalidations, toast show, alert, etc..)

Making the tests too coupled and hard to update

The route I took, basically ended with almost all module files with their own test files for their functions, without much integration. This simplifies some code changes on our side:

If we change the import flow, we just need to check that the taxonomy-menu feature is calling the correct function/component, and we can abstract the actual implementation of it

If we change the API, I just need to rewrite the API Tests, not the function/component that prepares the data for it

If we change the way we show success/error messages we just need to update the code in the Layout that handles it; not all functions that show success/error messages

We have pros/cons from both approaches, so I will happily follow your lead on the best way to do this for this project: combining the tests, a more unitary approach or a compromise between the two.

I understand your concerns. I'll highlight my reasons for recommending testing in a more combined way and perhaps @bradenmacdonald can also give his views on the same.

Re: breaking the "feature barrier". If you're importing one feature from another, you've already broken it. If you have three components, A, B and C, where B and C are under A, and B and C are importing from each other, then perhaps you can test at the level of A. Or you can look at reorganising so that there is no cross-importing. If both B and C are using the same component then perhaps those components should be moved to a common location from which both can import the component.

Duplicating tests: I think this is a good thing. More on this later.

More complex tests: I don't think the tests will be more complex. I think for the API responses you can define the responses once and then reuse those across multiple tests.

Making the tests too coupled: The tests are coupled if your code is coupled, not otherwise.

I think one big disadvantage of testing thing in isolation is that these are not isolated components. If you are testing 5 different components and they all include some common components, then it might seem like those common components are being tested 5 times, but I think they should be tested 5 times!

Here is an example. You create a shared component, let's call it TagList. This is now used by both TagListDropdown and TagListModal. At one point you are working on TagListModal and need to make changes to the core TagList component. You update it and TagListModal and they work great together. The TagList tests pass because they only test TagList. The TagListModal and TagListDropdown tests also pass because they both only test their component and mock the shared TagList component. In actuality if you were to go to a page that uses TagListDropdown, it would break, because the changes to TagList made them incompatible. If you used more integrated tests, that didn't use mocks then the tests would ideally fail in this scenario.

I think if two "features" of the app are completely isolated then it's definitely OK to have them be isolated in tests. However, the fact that the UI is in taxonomy-menu and the import code is in taxonomy-import is exactly the reason why they should be tested together, because they might both work and pass tests in isolation and be completely broken in practice.

Hi @xitij2000! Thank you for taking the time to provide a detailed response!

If you're importing one feature from another, you've already broken it.
I think it is okay to import from siblings from the ADR. I was concerned about putting the tests of feature A in feature B, but I got the overall idea. Maybe the taxonomy-menu here is not the best example (because it's just a menu and unlikely to fail), but your TagList example makes a lot of sense.

I will work to write the test this way for the following features.

Let me know if I should fix it here (and in the other PRs). I'm concerned about the budget, but if you think it is worth it, I'm okay with rewriting the tests to improve them.

Again, thank you for the explanation!

I tend to agree with @xitij2000 's overall philosophy here. I personally tend to write tests that use (coupled) components exactly as they are used in the app, because (A) it can find bugs with the actual integration of the components, like @xitij2000 explained with that example, (B) it's often much easier to do so than to figure out how to use mocks to isolate the components, and (C) I've spent a lot of time over the years hunting down and fixing test failures that were caused by the mocks, not the actual components themselves.

src/taxonomy/import-tags/data/utils.test.js

xitij2000 · 2023-12-28T08:09:12Z

src/taxonomy/taxonomy-menu/TaxonomyMenu.jsx

+  IconButton,
+} from '@edx/paragon';
+import { MoreVert } from '@edx/paragon/icons';
+import _ from 'lodash';


Load just the tool needed so that the whole of lodash doesn't need to be included in the bundle.

Suggested change

import _ from 'lodash';

import {omitBy} from 'lodash';

Done a3c40fd

xitij2000 · 2023-12-28T08:12:39Z

src/taxonomy/taxonomy-menu/TaxonomyMenu.jsx

+  const getTaxonomyMenuItems = () => {
+    let menuItems = {
+      import: {
+        title: intl.formatMessage(messages.importMenu),
+        action: () => importTaxonomyTags(taxonomy.id, intl),
+        // Hide import menu item if taxonomy is system defined or allows free text
+        hide: taxonomy.systemDefined || taxonomy.allowFreeText,
+      },
+      export: {
+        title: intl.formatMessage(messages.exportMenu),
+        action: exportModalOpen,
+      },
+      delete: {
+        title: intl.formatMessage(messages.deleteMenu),
+        action: deleteDialogOpen,
+        // Hide delete menu item if taxonomy is system defined
+        hide: taxonomy.systemDefined,
+      },
+    };
+
+    // Remove hidden menu items
+    menuItems = _.omitBy(menuItems, (value) => value.hide);
+
+    return menuItems;
+  };
+
+  const menuItems = getTaxonomyMenuItems();


Is there any advantage in defining this as a function, calling it in one place and using it in one place?

I think the value can just be used directly, unless this function is intended to be reusable and moved outside the component.

There is no need to be a function here. Refactored: a3c40fd
Also added types and fixed a bug

xitij2000 · 2023-12-28T08:46:12Z

src/taxonomy/taxonomy-menu/TaxonomyMenu.test.jsx

+    jest.clearAllMocks();
+  });
+
+  [true, false].forEach((iconMenu) => {


Use jest-each here: https://www.npmjs.com/package/jest-each

Done: a8b4197

xitij2000 · 2023-12-28T09:05:30Z

src/taxonomy/taxonomy-menu/TaxonomyMenu.test.jsx

+      const { getByTestId } = render(<TaxonomyMenuComponent iconMenu={iconMenu} />);
+
+      // Menu closed/doesn't exist yet
+      expect(() => getByTestId('taxonomy-menu')).toThrow();


I see this pattern in a use a lot and I think it a bit misleading. The error being thrown here is not by our code and as such is not expected.

What should be used instead is:

Suggested change

expect(() => getByTestId('taxonomy-menu')).toThrow();

expect(queryByTestId('taxonomy-menu')).not.toBeInTheDocument();

I think this better conveys our expectations.

Thank you @xitij2000! That's precisely what we want here.
Fixed: e9893dc

src/taxonomy/TaxonomyListPage.test.jsx

xitij2000 · 2023-12-29T04:50:14Z

src/taxonomy/TaxonomyListPage.test.jsx

+  it('calls the import taxonomy action when the import button is clicked', async () => {
    useIsTaxonomyListDataLoaded.mockReturnValue(true);
    useTaxonomyListDataResponse.mockReturnValue({
-      results: taxonomies,
+      results: [{
+        id: 1,
+        name: 'Taxonomy',
+        description: 'This is a description',
+      }],
    });
-    mockDeleteTaxonomy.mockImplementationOnce(async (params, callbacks) => {
-      callbacks.onSuccess();
+    await act(async () => {
+      const { getByTestId } = render(<RootWrapper />);
+      const importButton = getByTestId('taxonomy-import-button');
+      expect(importButton).toBeInTheDocument();
+      importButton.click();
+      expect(importTaxonomy).toHaveBeenCalled();
    });
-    const { getByTestId, getByLabelText } = render(<RootWrapper />);
-    fireEvent.click(getByTestId('test-delete-button'));
-    fireEvent.change(getByLabelText('Type DELETE to confirm'), { target: { value: 'DELETE' } });
-    fireEvent.click(getByTestId('delete-button'));
-
-    expect(mockDeleteTaxonomy).toBeCalledTimes(1);
-    expect(mockSetToastMessage).toBeCalledWith(`"${taxonomies[0].name}" deleted`);


Hmm... I think that is a recurring issue. I've made a suggestion for the test, other than that I guess we should leave it for now.

xitij2000 · 2023-12-29T05:19:06Z

src/taxonomy/import-tags/data/api.test.js

+import MockAdapter from 'axios-mock-adapter';
+import { initializeMockApp } from '@edx/frontend-platform';
+import { getAuthenticatedHttpClient } from '@edx/frontend-platform/auth';
+
+import { tagImportMock, taxonomyImportMock } from '../__mocks__';
+
+import {
+  getTaxonomyImportNewApiUrl,
+  getTagsImportApiUrl,
+  importNewTaxonomy,
+  importTags,
+} from './api';
+
+let axiosMock;
+
+describe('import taxonomy api calls', () => {
+  beforeEach(() => {
+    initializeMockApp({
+      authenticatedUser: {
+        userId: 3,
+        username: 'abc123',
+        administrator: true,
+        roles: [],
+      },
+    });
+    axiosMock = new MockAdapter(getAuthenticatedHttpClient());
+  });
+
+  afterEach(() => {
+    jest.clearAllMocks();
+  });
+
+  it('should call import new taxonomy', async () => {
+    axiosMock.onPost(getTaxonomyImportNewApiUrl()).reply(201, taxonomyImportMock);
+    const result = await importNewTaxonomy('Taxonomy name', 'Taxonomy description');
+
+    expect(axiosMock.history.post[0].url).toEqual(getTaxonomyImportNewApiUrl());
+    expect(result).toEqual(taxonomyImportMock);
+  });
+
+  it('should call import tags', async () => {
+    axiosMock.onPut(getTagsImportApiUrl(1)).reply(200, tagImportMock);
+    const result = await importTags(1);
+
+    expect(axiosMock.history.put[0].url).toEqual(getTagsImportApiUrl(1));
+    expect(result).toEqual(tagImportMock);
+  });
+});


I understand your concerns. I'll highlight my reasons for recommending testing in a more combined way and perhaps @bradenmacdonald can also give his views on the same.

Re: breaking the "feature barrier". If you're importing one feature from another, you've already broken it. If you have three components, A, B and C, where B and C are under A, and B and C are importing from each other, then perhaps you can test at the level of A. Or you can look at reorganising so that there is no cross-importing. If both B and C are using the same component then perhaps those components should be moved to a common location from which both can import the component.

Duplicating tests: I think this is a good thing. More on this later.

More complex tests: I don't think the tests will be more complex. I think for the API responses you can define the responses once and then reuse those across multiple tests.

Making the tests too coupled: The tests are coupled if your code is coupled, not otherwise.

I think one big disadvantage of testing thing in isolation is that these are not isolated components. If you are testing 5 different components and they all include some common components, then it might seem like those common components are being tested 5 times, but I think they should be tested 5 times!

Here is an example. You create a shared component, let's call it TagList. This is now used by both TagListDropdown and TagListModal. At one point you are working on TagListModal and need to make changes to the core TagList component. You update it and TagListModal and they work great together. The TagList tests pass because they only test TagList. The TagListModal and TagListDropdown tests also pass because they both only test their component and mock the shared TagList component. In actuality if you were to go to a page that uses TagListDropdown, it would break, because the changes to TagList made them incompatible. If you used more integrated tests, that didn't use mocks then the tests would ideally fail in this scenario.

I think if two "features" of the app are completely isolated then it's definitely OK to have them be isolated in tests. However, the fact that the UI is in taxonomy-menu and the import code is in taxonomy-import is exactly the reason why they should be tested together, because they might both work and pass tests in isolation and be completely broken in practice.

Co-authored-by: Kshitij Sobti <kshitij@sobti.in>

bradenmacdonald · 2024-01-04T19:23:53Z

@rpenido @xitij2000 Are we close to getting this merged? And the other two open PRs? We're quite behind schedule on this PR now.

xitij2000 · 2024-01-05T10:30:50Z

@rpenido @xitij2000 Are we close to getting this merged? And the other two open PRs? We're quite behind schedule on this PR now.

There seems to be one failing test. Once that is fixed this looks good to merge.

pomegranited · 2024-01-08T02:25:17Z

@xitij2000 @rpenido That test wasn't related to these changes -- merging latest master fixed the tests, so this should be ready to merge now?

xitij2000

One final small change and this is good to merge.

xitij2000 · 2024-01-08T06:54:13Z

src/taxonomy/taxonomy-menu/TaxonomyMenu.test.jsx

+import { QueryClient, QueryClientProvider } from '@tanstack/react-query';
+import { render, fireEvent, waitFor } from '@testing-library/react';
+import PropTypes from 'prop-types';
+import each from 'jest-each';


jest-each is included in jest, so you don't need to install the package or import is. Just do describe.each instead of each.describe

xitij2000 · 2024-01-08T06:54:30Z

src/taxonomy/taxonomy-menu/TaxonomyMenu.test.jsx

+  allowFreeText: false,
+};
+
+each([true, false]).describe('<TaxonomyMenu iconMenu=%s />', async (iconMenu) => {


Suggested change

each([true, false]).describe('<TaxonomyMenu iconMenu=%s />', async (iconMenu) => {

describe.each([true, false])('<TaxonomyMenu iconMenu=%s />', async (iconMenu) => {

xitij2000 · 2024-01-08T06:55:06Z

package.json

@@ -93,6 +93,7 @@
    "glob": "7.2.3",
    "husky": "7.0.4",
    "jest-canvas-mock": "^2.5.2",
+    "jest-each": "^29.7.0",


Suggested change

"jest-each": "^29.7.0",

xitij2000 · 2024-01-08T06:59:08Z

One final small change and this is good to merge.

To avoid another review pass I've just applied this change and will merge when tests pass.

openedx-webhooks · 2024-01-08T07:08:07Z

@rpenido 🎉 Your pull request was merged! Please take a moment to answer a two question survey so we can improve your experience in the future.

rpenido · 2024-01-08T13:06:52Z

To avoid another review pass I've just applied this change and will merge when tests pass.

Thank you @xitij2000!
I will update the other PRs with these changes.

…036] (#732) This PR improves the import tags functionality for existing taxonomies implemented at #675. Co-authored-by: Jillian <jill@opencraft.com> Co-authored-by: Braden MacDonald <mail@bradenm.com>

openedx-webhooks added the open-source-contribution PR author is not from Axim or 2U label Nov 8, 2023

feat: add import taxonomy feature

0c60c37

rpenido force-pushed the rpenido/fal-3532-import-taxonomy branch from da5e3f7 to 0c60c37 Compare November 9, 2023 12:34

rpenido changed the title ~~feat: add import taxonomy button~~ feat: add import taxonomy feature Nov 9, 2023

rpenido mentioned this pull request Nov 9, 2023

[temp] feat: add import taxonomy button open-craft/frontend-app-authoring#8

Closed

rpenido added 2 commits November 9, 2023 10:33

test: add api.test.js

c273af1

test: add import button click test

047530d

rpenido force-pushed the rpenido/fal-3532-import-taxonomy branch from f3a6bdf to 0bfd576 Compare November 9, 2023 17:59

test: add action.test.js

303e7ba

rpenido force-pushed the rpenido/fal-3532-import-taxonomy branch from ec7a119 to 303e7ba Compare November 9, 2023 20:16

rpenido and others added 4 commits November 9, 2023 19:54

test: add more tests to action.tests.js

3955e91

test: add more tests to action.tests.js 2

8176973

Merge branch 'openedx:master' into rpenido/fal-3532-import-taxonomy

4f31714

fix: import

df441fb

pomegranited approved these changes Nov 10, 2023

View reviewed changes

src/taxonomy/import-tags/index.js Outdated Show resolved Hide resolved

src/taxonomy/import-tags/data/api.test.js Outdated Show resolved Hide resolved

jest.config.js Outdated Show resolved Hide resolved

rpenido and others added 3 commits November 10, 2023 16:15

test: simplify import test

a0e92f0

Co-authored-by: Jillian <jill@opencraft.com>

fix: remove undefined var

dc8ed9e

refactor: rename actions.js -> utils.js

120154e

rpenido force-pushed the rpenido/fal-3532-import-taxonomy branch from 05bcd23 to 120154e Compare November 10, 2023 19:37

revert: change in the jest.config

318bbb7

rpenido marked this pull request as ready for review November 10, 2023 19:42

bradenmacdonald reviewed Nov 13, 2023

View reviewed changes

jristau1984 added the jira:2u We want an issue in the 2U Jira instance label Nov 14, 2023

This was referenced Dec 19, 2023

feat: refined ux update a taxonomy by downloading and uploading [FC-0036] #732

Merged

feat: assign taxonomy to organizations [FC-0036] #760

Merged

Merge branch 'master' into rpenido/fal-3532-import-taxonomy

fdc74c7

xitij2000 reviewed Dec 28, 2023

View reviewed changes

fix: change expect getBy toThrow pattern to queryBy not.toBeInDocument

e9893dc

rpenido force-pushed the rpenido/fal-3532-import-taxonomy branch from dddd441 to e9893dc Compare December 28, 2023 14:19

rpenido and others added 5 commits December 28, 2023 11:42

test: add jest-each

a8b4197

fix: remove getTaxonomyMenuItems function

a3c40fd

fix: remove tests from temp code

1a31aa9

Merge branch 'openedx:master' into rpenido/fal-3532-import-taxonomy

c6d39e5

test: improve coverage

583ab51

xitij2000 reviewed Dec 29, 2023

View reviewed changes

rpenido and others added 2 commits December 29, 2023 09:47

test: refactor import button test

151ba9e

Co-authored-by: Kshitij Sobti <kshitij@sobti.in>

fix: eslint

df9f2a0

rpenido added 2 commits January 4, 2024 17:13

Merge branch 'master' into rpenido/fal-3532-import-taxonomy

67c42cb

fix: errors fixing merge conflicts

793810f

Merge branch 'master' into rpenido/fal-3532-import-taxonomy

70954b3

pomegranited mentioned this pull request Jan 8, 2024

Use object permissions in Tagging frontend [FC-0036] open-craft/frontend-app-authoring#25

Closed

xitij2000 approved these changes Jan 8, 2024

View reviewed changes

fixup!: remove jest-each in favour of inbuilt functionality

68e820d

xitij2000 merged commit 6c0fc09 into openedx:master Jan 8, 2024
6 checks passed

kenclary mentioned this pull request Jan 9, 2024

fix: configure webpack to fix paid powerpaste plugin #783

Merged

pomegranited deleted the rpenido/fal-3532-import-taxonomy branch January 10, 2024 06:19

	expect(() => getByTestId('taxonomy-menu')).toThrow();
	expect(queryByTestId('taxonomy-menu')).not.toBeInTheDocument();

	each([true, false]).describe('<TaxonomyMenu iconMenu=%s />', async (iconMenu) => {
	describe.each([true, false])('<TaxonomyMenu iconMenu=%s />', async (iconMenu) => {

feat: add import taxonomy feature [FC-0036] #675

feat: add import taxonomy feature [FC-0036] #675

Conversation

rpenido commented Nov 8, 2023 • edited Loading

Description

Supporting Information

Testing instructions

Import a new taxonomy

Re-import tags to an existing taxonomy

openedx-webhooks commented Nov 8, 2023 • edited Loading

codecov bot commented Nov 8, 2023 • edited Loading

Codecov Report

rpenido commented Nov 10, 2023

pomegranited left a comment

Choose a reason for hiding this comment

pomegranited commented Nov 13, 2023

Choose a reason for hiding this comment

bradenmacdonald commented Nov 13, 2023

openedx-webhooks commented Nov 14, 2023

bradenmacdonald commented Nov 14, 2023

xitij2000 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rpenido Dec 28, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rpenido Dec 28, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rpenido Dec 28, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bradenmacdonald commented Jan 4, 2024

xitij2000 commented Jan 5, 2024

pomegranited commented Jan 8, 2024

xitij2000 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

xitij2000 commented Jan 8, 2024

openedx-webhooks commented Jan 8, 2024

rpenido commented Jan 8, 2024

rpenido commented Nov 8, 2023 •

edited

Loading

openedx-webhooks commented Nov 8, 2023 •

edited

Loading

codecov bot commented Nov 8, 2023 •

edited

Loading

rpenido Dec 28, 2023 •

edited

Loading

rpenido Dec 28, 2023 •

edited

Loading

rpenido Dec 28, 2023 •

edited

Loading