Repository navigation
feat: add update taxonomy orgs rest api [FC-0036] #33611
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
Changes from 8 commits
a0f9e66
c7b04ec
243585a
1933c87
1222c7d
0b16bc0
8c5ea99
79a95bc
e2e8f76
55d429a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -40,6 +40,7 @@ | |
|
|
||
| TAXONOMY_ORG_LIST_URL = "/api/content_tagging/v1/taxonomies/" | ||
| TAXONOMY_ORG_DETAIL_URL = "/api/content_tagging/v1/taxonomies/{pk}/" | ||
| TAXONOMY_ORG_UPDATE_ORG_URL = "/api/content_tagging/v1/taxonomies/{pk}/orgs/" | ||
| OBJECT_TAG_UPDATE_URL = "/api/content_tagging/v1/object_tags/{object_id}/?taxonomy={taxonomy_id}" | ||
| TAXONOMY_TEMPLATE_URL = "/api/content_tagging/v1/taxonomies/import/{filename}" | ||
|
|
||
|
|
@@ -454,7 +455,7 @@ def test_create_taxonomy(self, user_attr: str, expected_status: int) -> None: | |
|
|
||
| # Also checks if the taxonomy was associated with the org | ||
| if user_attr == "staffA": | ||
| assert TaxonomyOrg.objects.filter(taxonomy=response.data["id"], org=self.orgA).exists() | ||
| assert response.data["orgs"] == [self.orgA.short_name] | ||
|
|
||
|
|
||
| @ddt.ddt | ||
|
|
@@ -1044,6 +1045,204 @@ def _test_api_call(self, **kwargs) -> None: | |
| assert response.status_code == status.HTTP_404_NOT_FOUND | ||
|
|
||
|
|
||
| @skip_unless_cms | ||
| @ddt.ddt | ||
| class TestTaxonomyUpdateOrg(TestTaxonomyObjectsMixin, APITestCase): | ||
| """ | ||
| Test cases for updating orgs from taxonomies | ||
| """ | ||
|
|
||
| def test_update_org(self) -> None: | ||
| """ | ||
| Tests that taxonomy admin can add/remove orgs from a taxonomy | ||
| """ | ||
| url = TAXONOMY_ORG_UPDATE_ORG_URL.format(pk=self.tA1.pk) | ||
| self.client.force_authenticate(user=self.staff) | ||
|
|
||
| response = self.client.put(url, {"orgs": [self.orgB.short_name, self.orgX.short_name]}, format="json") | ||
| assert response.status_code == status.HTTP_200_OK | ||
|
|
||
| # Check that the orgs were updated | ||
| url = TAXONOMY_ORG_DETAIL_URL.format(pk=self.tA1.pk) | ||
| response = self.client.get(url) | ||
| assert response.data["orgs"] == [self.orgB.short_name, self.orgX.short_name] | ||
|
|
||
| def test_update_all_org(self) -> None: | ||
| """ | ||
| Tests that taxonomy admin can associate a taxonomy to all orgs | ||
| """ | ||
| url = TAXONOMY_ORG_UPDATE_ORG_URL.format(pk=self.tA1.pk) | ||
| self.client.force_authenticate(user=self.staff) | ||
|
|
||
| response = self.client.put(url, {"all_orgs": True}, format="json") | ||
| assert response.status_code == status.HTTP_200_OK | ||
|
|
||
| # Check that the orgs were updated | ||
| url = TAXONOMY_ORG_DETAIL_URL.format(pk=self.tA1.pk) | ||
| response = self.client.get(url) | ||
| assert response.data["orgs"] == [None] | ||
|
|
||
| def test_update_no_org(self) -> None: | ||
| """ | ||
| Tests that taxonomy admin can associate a taxonomy no orgs | ||
| """ | ||
| url = TAXONOMY_ORG_UPDATE_ORG_URL.format(pk=self.tA1.pk) | ||
| self.client.force_authenticate(user=self.staff) | ||
|
|
||
| response = self.client.put(url, {"orgs": []}, format="json") | ||
|
|
||
| assert response.status_code == status.HTTP_200_OK | ||
|
|
||
| # Check that the orgs were updated | ||
| url = TAXONOMY_ORG_DETAIL_URL.format(pk=self.tA1.pk) | ||
| response = self.client.get(url) | ||
| assert response.data["orgs"] == [] | ||
|
|
||
| @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"), | ||
| (None, None, "Using neither all_orgs or orgs parameter should throw error"), | ||
| (False, 'InvalidOrg', "Passing an invalid org should throw error"), | ||
| ) | ||
| @ddt.unpack | ||
| def test_update_org_invalid_inputs(self, all_orgs: bool, orgs: list[str], reason: str) -> None: | ||
| """ | ||
| Tests if passing both or none of all_orgs and orgs parameters throws error | ||
| """ | ||
| url = TAXONOMY_ORG_UPDATE_ORG_URL.format(pk=self.tA1.pk) | ||
| self.client.force_authenticate(user=self.staff) | ||
|
|
||
| # Set body cleaning empty values | ||
| body = {k: v for k, v in {"all_orgs": all_orgs, "orgs": orgs}.items() if v is not None} | ||
| response = self.client.put(url, body, format="json") | ||
| assert response.status_code == status.HTTP_400_BAD_REQUEST, reason | ||
|
|
||
| # Check that the orgs didn't change | ||
| url = TAXONOMY_ORG_DETAIL_URL.format(pk=self.tA1.pk) | ||
| response = self.client.get(url) | ||
| assert response.data["orgs"] == [self.orgA.short_name] | ||
|
|
||
| def test_update_org_system_defined(self) -> None: | ||
| """ | ||
| Tests that is not possible to change the orgs associated with a system defined taxonomy | ||
| """ | ||
| url = TAXONOMY_ORG_UPDATE_ORG_URL.format(pk=self.st1.pk) | ||
| self.client.force_authenticate(user=self.staff) | ||
|
|
||
| response = self.client.put(url, {"orgs": [self.orgA.short_name]}, format="json") | ||
| assert response.status_code in [status.HTTP_403_FORBIDDEN, status.HTTP_400_BAD_REQUEST] | ||
|
|
||
| # 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] | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This doesn't have to block merging, but I don't think it's very nice to send 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.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Great suggestion @pomegranited! |
||
|
|
||
| @ddt.data( | ||
| "staffA", | ||
| "content_creatorA", | ||
| "instructorA", | ||
| "library_staffA", | ||
| "course_instructorA", | ||
| "course_staffA", | ||
| "library_userA", | ||
| ) | ||
| def test_update_org_no_perm(self, user_attr: str) -> None: | ||
| """ | ||
| Tests that only taxonomy admins can associate orgs to taxonomies | ||
| """ | ||
| url = TAXONOMY_ORG_UPDATE_ORG_URL.format(pk=self.tA1.pk) | ||
| user = getattr(self, user_attr) | ||
| self.client.force_authenticate(user=user) | ||
|
|
||
| response = self.client.put(url, {"orgs": []}, format="json") | ||
| assert response.status_code == status.HTTP_403_FORBIDDEN | ||
|
|
||
| # Check that the orgs didn't change | ||
| url = TAXONOMY_ORG_DETAIL_URL.format(pk=self.tA1.pk) | ||
| response = self.client.get(url) | ||
| assert response.data["orgs"] == [self.orgA.short_name] | ||
|
|
||
| def test_update_org_check_permissions_orgA(self) -> None: | ||
| """ | ||
| Tests that adding an org to a taxonomy allow org level admins to edit it | ||
| """ | ||
| url = TAXONOMY_ORG_DETAIL_URL.format(pk=self.tB1.pk) | ||
| self.client.force_authenticate(user=self.staffA) | ||
|
|
||
| response = self.client.put(url, {"name": "new name"}, format="json") | ||
|
|
||
| # User staffA can't update metadata from a taxonomy from orgB | ||
| assert response.status_code == status.HTTP_404_NOT_FOUND | ||
|
|
||
| url = TAXONOMY_ORG_UPDATE_ORG_URL.format(pk=self.tB1.pk) | ||
| self.client.force_authenticate(user=self.staff) | ||
|
|
||
| # Add the taxonomy tB1 to orgA | ||
| response = self.client.put(url, {"orgs": [self.orgA.short_name]}, format="json") | ||
|
|
||
| url = TAXONOMY_ORG_DETAIL_URL.format(pk=self.tB1.pk) | ||
| self.client.force_authenticate(user=self.staffA) | ||
|
|
||
| response = self.client.put(url, {"name": "new name"}, format="json") | ||
|
|
||
| # Now staffA can change the metadata from a tB1 because it's associated with orgA | ||
| assert response.status_code == status.HTTP_200_OK | ||
|
|
||
| def test_update_org_check_permissions_all_orgs(self) -> None: | ||
| """ | ||
| Tests that adding an org to all orgs only let taxonomy global admins to edit it | ||
| """ | ||
| url = TAXONOMY_ORG_DETAIL_URL.format(pk=self.tA1.pk) | ||
| self.client.force_authenticate(user=self.staffA) | ||
|
|
||
| response = self.client.put(url, {"name": "new name"}, format="json") | ||
|
|
||
| # User staffA can update metadata from a taxonomy from orgA | ||
| assert response.status_code == status.HTTP_200_OK | ||
|
|
||
| url = TAXONOMY_ORG_UPDATE_ORG_URL.format(pk=self.tB1.pk) | ||
| self.client.force_authenticate(user=self.staff) | ||
|
|
||
| # Add the taxonomy tA1 to all orgs | ||
| response = self.client.put(url, {"all_orgs": True}, format="json") | ||
|
|
||
| url = TAXONOMY_ORG_DETAIL_URL.format(pk=self.tB1.pk) | ||
| self.client.force_authenticate(user=self.staffA) | ||
|
|
||
| response = self.client.put(url, {"name": "new name"}, format="json") | ||
|
|
||
| # Now staffA can't change the metadata from a tA1 because only global taxonomy admins can edit all orgs | ||
| # taxonomies | ||
| assert response.status_code == status.HTTP_403_FORBIDDEN | ||
|
|
||
| def test_update_org_check_permissions_no_orgs(self) -> None: | ||
| """ | ||
| Tests that remove all orgs from a taxonomy only let taxonomy global admins to edit it | ||
| """ | ||
| url = TAXONOMY_ORG_DETAIL_URL.format(pk=self.tA1.pk) | ||
| self.client.force_authenticate(user=self.staffA) | ||
|
|
||
| response = self.client.put(url, {"name": "new name"}, format="json") | ||
|
|
||
| # User staffA can update metadata from a taxonomy from orgA | ||
| assert response.status_code == status.HTTP_200_OK | ||
|
|
||
| url = TAXONOMY_ORG_UPDATE_ORG_URL.format(pk=self.tB1.pk) | ||
| self.client.force_authenticate(user=self.staff) | ||
|
|
||
| # Remove all orgs from tA1 | ||
| response = self.client.put(url, {"orgs": []}, format="json") | ||
|
|
||
| url = TAXONOMY_ORG_DETAIL_URL.format(pk=self.tB1.pk) | ||
| self.client.force_authenticate(user=self.staffA) | ||
|
|
||
| response = self.client.put(url, {"name": "new name"}, format="json") | ||
|
|
||
| # Now staffA can't change the metadata from a tA1 because only global taxonomy admins can edit no orgs | ||
| # taxonomies | ||
| assert response.status_code == status.HTTP_404_NOT_FOUND | ||
|
|
||
|
|
||
| class TestObjectTagMixin(TestTaxonomyObjectsMixin): | ||
| """ | ||
| Sets up data for testing ObjectTags. | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This case looks valid to me.. How do we toggle from
all_orgs = Truetoall_orgs = False?Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You must pass the
all_orgs=Falseand an empty orgs (orgs=[]) array. If you think that omittingorgsshould have the same effect, let me know!Personally, I prefer the explicitly
orgs=[]way.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, as you wish.