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

rpenido · 2023-12-20T23:53:53Z

Description

This PR adds the UI to assign organizations to a Taxonomy.

Supporting Information

Closes:
- [Tagging] Assign taxonomy to organizations modular-learning#117
Depends on:
- feat: add import taxonomy feature [FC-0036] #675

Testing instructions

Go to the taxonomy list page (http://localhost:2001/taxonomies)
Click on the Manage Organizations menu from an existing taxonomy card
You will be prompted with a form to assign Organizations to the taxonomy.
Assign an org to the taxonomy and click on Save
Click on the Manage Organizations menu from the taxonomy and check if the org was assigned
Use the checkbox to Assign to all organizations. The other fields must be disabled. Click on Save
Click on the Manage Organizations menu from the taxonomy and check if the Taxonomy was assigned to all orgs
Uncheck the Assign to all organizations, leave the Taxonomy without assigned orgs and click on Save
A dialog will ask for confirmation if you leave a Taxonomy without associated orgs
Click on the Manage Organizations menu from the taxonomy and check if the Taxonomy has no assigned orgs

The feature can also be accessed from the Actions menu on the Taxonomy Details page.

Private-ref:

FAL-3543

Co-authored-by: Jillian <[email protected]>

* feat: import tags to existing taxonomy * feat: add re-import menu to taxonomy detail * test: add tests * fix: clean debug code Co-authored-by: Jillian <[email protected]> --------- Co-authored-by: Jillian <[email protected]>

* refactor: merges TaxonomyCardMenu and TaxonomyDetailMenu * refactor: change menu item list

…-assign-taxonomy-to-organizations

openedx-webhooks · 2023-12-20T23:53:58Z

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-12-21T00:02:55Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (138f1d2) 89.62% compared to head (aa1031e) 89.82%.
Report is 10 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #760      +/-   ##
==========================================
+ Coverage   89.62%   89.82%   +0.19%     
==========================================
  Files         499      504       +5     
  Lines        8089     8276     +187     
  Branches     1708     1745      +37     
==========================================
+ Hits         7250     7434     +184     
- Misses        812      815       +3     
  Partials       27       27

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

…zations

rpenido · 2024-01-08T18:27:03Z

Hi @xitij2000! I rebased this PR and it is ready to review!

xitij2000

Looks good! Just a few minor comments.

xitij2000 · 2024-01-11T09:13:45Z

src/taxonomy/manage-orgs/ManageOrgsModal.jsx

+        isOpen={isOpen}
+        onClose={onClose}
+        size="lg"
+        disabled={isDialogDisabled}


I don't think disabled is a valid property on ModalDialog.

Removed: 3cf5758

xitij2000 · 2024-01-11T10:06:46Z

src/taxonomy/manage-orgs/ManageOrgsModal.jsx

+        if (setToastMessage) {
+          setToastMessage(intl.formatMessage(messages.assignOrgsSuccess));
+        }
+      } catch (/** @type {any} */ error) {


I don't think this type annotation is helpful, unless it's needed to make some tool happy it's better to remove it or enhance it to provide better insight.

Removed: 3cf5758

xitij2000 · 2024-01-11T13:14:06Z

src/taxonomy/manage-orgs/ManageOrgsModal.jsx

+  useEffect(() => {
+    if (selectedOrgs) {
+      // This is a hack to force the Form.Autosuggest to clear its value after a selection is made.
+      const inputRef = /** @type {null|HTMLInputElement} */ (document.querySelector('.pgn__form-group input'));
+      if (inputRef) {
+        //  @ts-ignore
+        inputRef.value = null;
+        const event = new Event('change', { bubbles: true });
+        inputRef.dispatchEvent(event);
+      }
+    }
+  }, [selectedOrgs]);
+


This seems quite hacky, but I'm not sure what alternative there is.

That said, I think the selector here is a bit broad, I think you should scope this to .manage_orgs.

Added .manage-orgs: 3cf5758

There is some effort to refactor the Autosuggest component here: openedx/paragon#2899
But for now, this is what we have 😦

Good to know there some work towards a better alternative on the Paragon front!

xitij2000 · 2024-01-11T13:21:22Z

src/taxonomy/data/apiHooks.test.jsx

+describe('useTaxonomyDetailDataStatus', () => {
+  it('should return status values', () => {
+    const status = {
+      isError: false,
+      error: undefined,
+      isFetched: true,
+      isSuccess: true,
+    };
+
+    useQuery.mockReturnValueOnce(status);
+
+    const result = useTaxonomyDetailDataStatus(0);
+
+    expect(result).toEqual(status);
+  });
+});
+
+describe('useTaxonomyDetailDataResponse', () => {
+  it('should return data when status is success', () => {
+    useQuery.mockReturnValueOnce({ isSuccess: true, data: 'data' });
+
+    const result = useTaxonomyDetailDataResponse();
+
+    expect(result).toEqual('data');
+  });
+
+  it('should return undefined when status is not success', () => {
+    useQuery.mockReturnValueOnce({ isSuccess: false });
+
+    const result = useTaxonomyDetailDataResponse();
+
+    expect(result).toBeUndefined();
+  });
+});


These tests are not testing anything, please remove them in favour of testing the component directly.

Done: aa1031e

xitij2000 · 2024-01-11T13:30:42Z

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

+    fireEvent.click(getByTestId('taxonomy-menu-manageOrgs'));
+
+    // Modal opened
+    await waitFor(() => expect(getByText('Assign to organizations')).toBeInTheDocument());


Suggested change

await waitFor(() => expect(getByText('Assign to organizations')).toBeInTheDocument());

expect(await findByText('Assign to organizations')).toBeInTheDocument();

Done: 3cf5758

xitij2000 · 2024-01-11T13:30:54Z

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

+    fireEvent.click(getByText('Cancel'));
+
+    // Modal closed
+    expect(() => getByText('Assign to organizations')).toThrow();


Suggested change

expect(() => getByText('Assign to organizations')).toThrow();

expect(queryByText('Assign to organizations')).not.toBeInTheDocument();

Done: 3cf5758

rpenido · 2024-01-11T23:26:39Z

Thank you for your review @xitij2000.

Let me know if I missed something!

openedx-webhooks · 2024-01-12T07:41:00Z

@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 and others added 30 commits November 9, 2023 09:34

feat: add import taxonomy feature

0c60c37

test: add api.test.js

c273af1

test: add import button click test

047530d

test: add action.test.js

303e7ba

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

test: simplify import test

a0e92f0

Co-authored-by: Jillian <[email protected]>

fix: remove undefined var

dc8ed9e

refactor: rename actions.js -> utils.js

120154e

revert: change in the jest.config

318bbb7

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

a8c2fd5

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

70ac2a7

chore: trigger CD/CI

adacf23

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

5983953

feat: import tags to existing taxonomy

7756c7f

* feat: import tags to existing taxonomy * feat: add re-import menu to taxonomy detail * test: add tests * fix: clean debug code Co-authored-by: Jillian <[email protected]> --------- Co-authored-by: Jillian <[email protected]>

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

4202c27

refactor: merges TaxonomyCardMenu and TaxonomyDetailMenu (#13)

6bcd924

* refactor: merges TaxonomyCardMenu and TaxonomyDetailMenu * refactor: change menu item list

refactor: improve menu organization

f318dfc

fix: types

204b2b9

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

5e0f599

refactor: merging conflicts

f888ecf

refactor: change export syntax

3c8da66

fix: eslint

40479c1

fix: export TaxonomyDetail

46e5d1b

fix: eslint

db8ba58

feat: add manage-orgs modal

a8b82cf

fix: menu layout and input clear

6484bdc

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

8fe8c07

rpenido added 12 commits December 19, 2023 15:15

fix: merge conflicts

5aed312

style: cleaning code and fix lint

1002fc6

Merge branch 'rpenido/fal-3532-import-taxonomy' into rpenido/fal-3543…

30890b5

…-assign-taxonomy-to-organizations

fix: disable element if is allOrg

1e00d27

test: add tests

f962b29

style: add comment about modal style to allow dropdown overflow

9fbe76e

fix: add spacing

aca5ce5

fix: dont pass null children

2bf4c53

fix: cleaning code

6054020

test: add tests

44d2428

test: add more tests

ca38b43

fix: typo

5d2b314

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

xitij2000 self-requested a review December 28, 2023 10:16

rpenido added 2 commits January 8, 2024 14:53

Merge branch 'master' into rpenido/fal-3543-assign-taxonomy-to-organi…

25fb779

…zations

test: fix test

56e6c1e

rpenido mentioned this pull request Jan 8, 2024

feat: assign taxonomy to organizations [TEMP] open-craft/frontend-app-authoring#23

Closed

rpenido marked this pull request as ready for review January 8, 2024 18:07

fix: console warnings and disable dialog overlay

bc966b4

xitij2000 approved these changes Jan 11, 2024

View reviewed changes

rpenido added 2 commits January 11, 2024 13:19

fix: changes from review

3cf5758

test: update tests from review

aa1031e

rpenido force-pushed the rpenido/fal-3543-assign-taxonomy-to-organizations branch from b2745e1 to aa1031e Compare January 11, 2024 23:13

xitij2000 merged commit 1fef358 into openedx:master Jan 12, 2024
6 checks passed

xitij2000 deleted the rpenido/fal-3543-assign-taxonomy-to-organizations branch January 12, 2024 07:41

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

rpenido commented Dec 20, 2023 •

edited

Loading

openedx-webhooks commented Dec 20, 2023 •

edited

Loading

codecov bot commented Dec 21, 2023 •

edited

Loading

rpenido commented Jan 8, 2024

xitij2000 left a comment

xitij2000 Jan 11, 2024

rpenido Jan 11, 2024

xitij2000 Jan 11, 2024

rpenido Jan 11, 2024

xitij2000 Jan 11, 2024

rpenido Jan 11, 2024

xitij2000 Jan 12, 2024

xitij2000 Jan 11, 2024

rpenido Jan 11, 2024

xitij2000 Jan 11, 2024

rpenido Jan 11, 2024

xitij2000 Jan 11, 2024

rpenido Jan 11, 2024

rpenido commented Jan 11, 2024

openedx-webhooks commented Jan 12, 2024

	await waitFor(() => expect(getByText('Assign to organizations')).toBeInTheDocument());
	expect(await findByText('Assign to organizations')).toBeInTheDocument();

	expect(() => getByText('Assign to organizations')).toThrow();
	expect(queryByText('Assign to organizations')).not.toBeInTheDocument();

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

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

Conversation

rpenido commented Dec 20, 2023 • edited Loading

Description

Supporting Information

Testing instructions

openedx-webhooks commented Dec 20, 2023 • edited Loading

codecov bot commented Dec 21, 2023 • edited Loading

Codecov Report

rpenido 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

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 commented Jan 11, 2024

openedx-webhooks commented Jan 12, 2024

rpenido commented Dec 20, 2023 •

edited

Loading

openedx-webhooks commented Dec 20, 2023 •

edited

Loading

codecov bot commented Dec 21, 2023 •

edited

Loading