feat: add update taxonomy orgs rest api [FC-0036] by rpenido · Pull Request #33611 · openedx/openedx-platform

rpenido · 2023-10-27T20:19:19Z

Description

Add a rest api endpoint to allow taxonomy admins to change the orgs that are linked to a taxonomy

This also adds the orgs from a taxonomy in the serializer used in the list and detail actions.

Supporting Information

Closes [Tagging] REST API to assign a taxonomy to organizations modular-learning#141

Testing instructions

Please ensure that the tests cover the expected behavior of the view as described in the related issue.

Private-ref: FAL-3542

openedx-webhooks · 2023-10-27T20:19:24Z

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.

pomegranited

Looks good @rpenido ! I've left some questions about the tests, but let me know if you disagree.

pomegranited · 2023-10-31T05:04:31Z

+
+    @ddt.data(
+        (True, ["orgX"], "Using both all_orgs and orgs parameters should throw error"),
+        (False, None, "Using neither all_orgs or orgs parameter should throw error"),


This case looks valid to me.. How do we toggle from all_orgs = True to all_orgs = False?

You must pass the all_orgs=False and an empty orgs (orgs=[]) array. If you think that omitting orgs should have the same effect, let me know!

Personally, I prefer the explicitly orgs=[] way.

Sure, as you wish.

…nomy-to-organizations

open-craft-grove · 2023-10-31T19:22:54Z

Sandbox deployment successful.

Sandbox LMS is available at pr-33611-139931.staging.do.opencraft.hosting
Sandbox Studio is available at studio.pr-33611-139931.staging.do.opencraft.hosting

open-craft-grove · 2023-11-01T14:15:34Z

Sandbox update request received. Deployment will start soon.

open-craft-grove · 2023-11-01T14:19:24Z

Sandbox deployment started.

open-craft-grove · 2023-11-01T21:01:00Z

Sandbox update request received. Deployment will start soon.

open-craft-grove · 2023-11-01T21:05:11Z

Sandbox deployment started.

open-craft-grove · 2023-11-01T21:34:09Z

Sandbox deployment successful.

Sandbox LMS is available at pr-33611-139931.staging.do.opencraft.hosting
Sandbox Studio is available at studio.pr-33611-139931.staging.do.opencraft.hosting

open-craft-grove · 2023-11-01T22:47:17Z

Sandbox deployment successful.

Sandbox LMS is available at pr-33611-139931.staging.do.opencraft.hosting
Sandbox Studio is available at studio.pr-33611-139931.staging.do.opencraft.hosting

pomegranited

👍 Looks good, thank you @rpenido !

I ran this code on my devstack, but am relying on the tests to verify the actual functionality.
I read through the code
~~I checked for accessibility issues~~ N/A
Includes documentation
Commit structure follows OEP-0051

…nomy-to-organizations

bradenmacdonald · 2023-11-03T20:55:49Z

If @pomegranited is happy with this, I'll approve and merge on Monday.

pomegranited · 2023-11-05T23:44:38Z

+        # Check that the orgs didn't change
+        url = TAXONOMY_ORG_DETAIL_URL.format(pk=self.st1.pk)
+        response = self.client.get(url)
+        assert response.data["orgs"] == [None]


This doesn't have to block merging, but I don't think it's very nice to send ["orgs"] == [None] to the frontend.

We in the backend know this means "all orgs", but the frontend shouldn't have to know this. I'd rather we sent an explicit "all_orgs" flag with the response to indicate this case.

Great suggestion @pomegranited!
Done here: e2e8f76

…nomy-to-organizations

openedx-webhooks · 2023-11-07T19:24:46Z

@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.

open-craft-grove · 2023-11-07T19:45:43Z

Sandbox update request received. Deployment will start soon.

edx-pipeline-bot · 2023-11-07T20:28:25Z

2U Release Notice: This PR has been deployed to the edX staging environment in preparation for a release to production.

edx-pipeline-bot · 2023-11-07T20:51:39Z

2U Release Notice: This PR has been deployed to the edX production environment.

edx-pipeline-bot · 2023-11-07T20:51:39Z

2U Release Notice: This PR has been deployed to the edX production environment.

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

rpenido force-pushed the rpenido/fal-3542-rest-api-to-assign-a-taxonomy-to-organizations branch 3 times, most recently from a2f5f14 to a1a86b1 Compare October 27, 2023 21:00

feat: add update taxonomy orgs rest api

a0f9e66

rpenido force-pushed the rpenido/fal-3542-rest-api-to-assign-a-taxonomy-to-organizations branch from a1a86b1 to a0f9e66 Compare October 27, 2023 21:48

pomegranited requested changes Oct 31, 2023

View reviewed changes

rpenido marked this pull request as ready for review October 31, 2023 18:31

Merge branch 'master' into rpenido/fal-3542-rest-api-to-assign-a-taxo…

c7b04ec

…nomy-to-organizations

test: add permission change test

243585a

rpenido force-pushed the rpenido/fal-3542-rest-api-to-assign-a-taxonomy-to-organizations branch from d0fc470 to 243585a Compare November 1, 2023 20:46

test: add new permissions change test

1933c87

pomegranited approved these changes Nov 2, 2023

View reviewed changes

bradenmacdonald reviewed Nov 2, 2023

View reviewed changes

Comment thread openedx/core/djangoapps/content_tagging/rest_api/v1/views.py Outdated

rpenido added 2 commits November 3, 2023 09:02

docs: fix wrong docstring

1222c7d

Merge branch 'master' into rpenido/fal-3542-rest-api-to-assign-a-taxo…

0b16bc0

…nomy-to-organizations

rpenido marked this pull request as draft November 3, 2023 17:19

feat: add org to taxonomy serializer

8c5ea99

rpenido marked this pull request as ready for review November 3, 2023 20:41

style: fix pylint

79a95bc

pomegranited reviewed Nov 5, 2023

View reviewed changes

rpenido added 2 commits November 6, 2023 10:34

feat: add all_orgs field

e2e8f76

Merge branch 'master' into rpenido/fal-3542-rest-api-to-assign-a-taxo…

55d429a

…nomy-to-organizations

bradenmacdonald added the FC Relates to an Axim Funded Contribution project label Nov 7, 2023

bradenmacdonald changed the title ~~feat: add update taxonomy orgs rest api~~ feat: add update taxonomy orgs rest api [FC-0036] Nov 7, 2023

bradenmacdonald approved these changes Nov 7, 2023

View reviewed changes

bradenmacdonald merged commit 4b4e370 into openedx:master Nov 7, 2023

bradenmacdonald deleted the rpenido/fal-3542-rest-api-to-assign-a-taxonomy-to-organizations branch November 7, 2023 19:24

Uh oh!

Conversation

rpenido commented Oct 27, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Supporting Information

Testing instructions

Uh oh!

openedx-webhooks commented Oct 27, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pomegranited left a comment

Choose a reason for hiding this comment

Uh oh!

pomegranited Oct 31, 2023

Choose a reason for hiding this comment

Uh oh!

rpenido Oct 31, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pomegranited Oct 31, 2023

Choose a reason for hiding this comment

Uh oh!

Uh oh!

open-craft-grove commented Oct 31, 2023

Uh oh!

open-craft-grove commented Nov 1, 2023

Uh oh!

open-craft-grove commented Nov 1, 2023

Uh oh!

open-craft-grove commented Nov 1, 2023

Uh oh!

open-craft-grove commented Nov 1, 2023

Uh oh!

open-craft-grove commented Nov 1, 2023

Uh oh!

open-craft-grove commented Nov 1, 2023

Uh oh!

pomegranited left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

bradenmacdonald commented Nov 3, 2023

Uh oh!

pomegranited Nov 5, 2023

Choose a reason for hiding this comment

Uh oh!

rpenido Nov 6, 2023

Choose a reason for hiding this comment

Uh oh!

openedx-webhooks commented Nov 7, 2023

Uh oh!

open-craft-grove commented Nov 7, 2023

Uh oh!

edx-pipeline-bot commented Nov 7, 2023

Uh oh!

edx-pipeline-bot commented Nov 7, 2023

Uh oh!

edx-pipeline-bot commented Nov 7, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

rpenido commented Oct 27, 2023 •

edited

Loading

openedx-webhooks commented Oct 27, 2023 •

edited

Loading

rpenido Oct 31, 2023 •

edited

Loading