diff --git a/cms/djangoapps/contentstore/helpers.py b/cms/djangoapps/contentstore/helpers.py index 8b5ad7030228..706a20273784 100644 --- a/cms/djangoapps/contentstore/helpers.py +++ b/cms/djangoapps/contentstore/helpers.py @@ -20,8 +20,6 @@ from xmodule.modulestore.django import modulestore from cms.djangoapps.models.settings.course_grading import CourseGradingModel -from common.djangoapps.student import auth -from common.djangoapps.student.roles import CourseCreatorRole, OrgContentCreatorRole import openedx.core.djangoapps.content_staging.api as content_staging_api from .utils import reverse_course_url, reverse_library_url, reverse_usage_url @@ -377,15 +375,3 @@ def is_item_in_course_tree(item): ancestor = ancestor.get_parent() return ancestor is not None - - -def is_content_creator(user, org): - """ - Check if the user has the role to create content. - - This function checks if the User has role to create content - or if the org is supplied, it checks for Org level course content - creator. - """ - return (auth.user_has_role(user, CourseCreatorRole()) or - auth.user_has_role(user, OrgContentCreatorRole(org=org))) diff --git a/cms/djangoapps/contentstore/views/course.py b/cms/djangoapps/contentstore/views/course.py index 99669e5a7276..fe70ba32e08d 100644 --- a/cms/djangoapps/contentstore/views/course.py +++ b/cms/djangoapps/contentstore/views/course.py @@ -45,7 +45,8 @@ has_course_author_access, has_studio_read_access, has_studio_write_access, - has_studio_advanced_settings_access + has_studio_advanced_settings_access, + is_content_creator, ) from common.djangoapps.student.roles import ( CourseInstructorRole, @@ -102,7 +103,6 @@ update_course_details, ) from .component import ADVANCED_COMPONENT_TYPES -from ..helpers import is_content_creator from cms.djangoapps.contentstore.xblock_services.xblock_service import ( create_xblock_info, ) diff --git a/cms/djangoapps/contentstore/views/library.py b/cms/djangoapps/contentstore/views/library.py index efcad6e6035d..8b38fbb0763f 100644 --- a/cms/djangoapps/contentstore/views/library.py +++ b/cms/djangoapps/contentstore/views/library.py @@ -30,7 +30,8 @@ STUDIO_VIEW_USERS, get_user_permissions, has_studio_read_access, - has_studio_write_access + has_studio_write_access, + is_content_creator, ) from common.djangoapps.student.roles import ( CourseInstructorRole, @@ -42,7 +43,6 @@ from ..config.waffle import REDIRECT_TO_LIBRARY_AUTHORING_MICROFRONTEND from ..utils import add_instructor, reverse_library_url from .component import CONTAINER_TEMPLATES, get_component_templates -from ..helpers import is_content_creator from cms.djangoapps.contentstore.xblock_services.xblock_service import create_xblock_info from .user import user_with_role diff --git a/cms/envs/common.py b/cms/envs/common.py index 459c24de329d..f0e99423b729 100644 --- a/cms/envs/common.py +++ b/cms/envs/common.py @@ -1756,6 +1756,11 @@ # API Documentation 'drf_yasg', + # Tagging + 'openedx_tagging.core.tagging.apps.TaggingConfig', + 'openedx.features.content_tagging', + + # Features 'openedx.features.course_duration_limits', 'openedx.features.content_type_gating', 'openedx.features.discounts', diff --git a/common/djangoapps/student/auth.py b/common/djangoapps/student/auth.py index f8e45ce6ba6c..47aef5cf6385 100644 --- a/common/djangoapps/student/auth.py +++ b/common/djangoapps/student/auth.py @@ -150,6 +150,18 @@ def has_studio_read_access(user, course_key): return bool(STUDIO_VIEW_CONTENT & get_user_permissions(user, course_key)) +def is_content_creator(user, org): + """ + Check if the user has the role to create content. + + This function checks if the User has role to create content + or if the org is supplied, it checks for Org level course content + creator. + """ + return (user_has_role(user, CourseCreatorRole()) or + user_has_role(user, OrgContentCreatorRole(org=org))) + + def add_users(caller, role, *users): """ The caller requests adding the given users to the role. Checks that the caller diff --git a/lms/envs/common.py b/lms/envs/common.py index 16382b6175a8..755a31479ade 100644 --- a/lms/envs/common.py +++ b/lms/envs/common.py @@ -3216,6 +3216,10 @@ def _make_locale_paths(settings): # pylint: disable=missing-function-docstring # Course Goals 'lms.djangoapps.course_goals.apps.CourseGoalsConfig', + # Tagging + 'openedx_tagging.core.tagging.apps.TaggingConfig', + 'openedx.features.content_tagging', + # Features 'openedx.features.calendar_sync', 'openedx.features.course_bookmarks', diff --git a/openedx/features/content_tagging/__init__.py b/openedx/features/content_tagging/__init__.py new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/openedx/features/content_tagging/admin.py b/openedx/features/content_tagging/admin.py new file mode 100644 index 000000000000..f146a43187d4 --- /dev/null +++ b/openedx/features/content_tagging/admin.py @@ -0,0 +1,19 @@ +""" Tagging app admin """ +from django.contrib import admin + +from .models import ContentTaxonomy + + +class ContentTaxonomyOrgAdmin(admin.TabularInline): + model = ContentTaxonomy.org_owners.through + + +class ContentTaxonomyAdmin(admin.ModelAdmin): + """ + Admin form for the content taxonomy table. + """ + + inlines = (ContentTaxonomyOrgAdmin,) + + +admin.site.register(ContentTaxonomy, ContentTaxonomyAdmin) diff --git a/openedx/features/content_tagging/api.py b/openedx/features/content_tagging/api.py new file mode 100644 index 000000000000..a80607dba9dd --- /dev/null +++ b/openedx/features/content_tagging/api.py @@ -0,0 +1,83 @@ +""" +Content Tagging APIs +""" +from typing import List + +import openedx_tagging.core.tagging.api as oel_tagging +from django.db.models import QuerySet +from organizations.models import Organization + +from .models import ContentTaxonomy + + +def create_taxonomy( + name, + org_owners: List[Organization] = None, + description=None, + enabled=True, + required=False, + allow_multiple=False, + allow_free_text=False, +) -> ContentTaxonomy: + """ + Creates, saves, and returns a new ContentTaxonomy with the given attributes. + + If `org_owners` is empty/None, then the returned taxonomy is enabled for all organizations. + """ + taxonomy = ContentTaxonomy.objects.create( + name=name, + description=description, + enabled=enabled, + required=required, + allow_multiple=allow_multiple, + allow_free_text=allow_free_text, + ) + if org_owners: + set_taxonomy_org_owners(taxonomy, org_owners) + return taxonomy + + +def set_taxonomy_org_owners( + taxonomy: ContentTaxonomy, + org_owners: List[Organization] = None, +) -> ContentTaxonomy: + """ + Updates the list of org_owners on the given taxonomy. + + If `org_owners` is empty/None, then the returned taxonomy is enabled for all organizations. + """ + if org_owners: + taxonomy.org_owners.set(org_owners) + else: + taxonomy.org_owners.clear() + return taxonomy + + +def get_taxonomies_for_org(org_owner: Organization = None, enabled=True) -> QuerySet: + """ + Returns a queryset containing the enabled ContentTaxonomies owned by the given org, sorted by name. + + If you want all Taxonomies, not just ContentTaxonomies, use the get_taxonomies API method, and use `enabled_for_org` + on any returned ContentTaxonomies to filter out by org ownership. + + If you want the disabled ContentTaxonomies, pass enabled=False. + If you want all ContentTaxonomies (both enabled and disabled), pass enabled=None. + """ + return ( + ContentTaxonomy.objects.filter_enabled( + org_owner, + enabled, + ) + .order_by("name", "id") + .select_subclasses() + ) + + +# Expose the oel_tagging APIs that we haven't overridden here: + +get_taxonomies = oel_tagging.get_taxonomies +get_taxonomy = oel_tagging.get_taxonomy +get_tags = oel_tagging.get_tags +resync_object_tags = oel_tagging.resync_object_tags +get_object_tags = oel_tagging.get_object_tags +tag_object = oel_tagging.tag_object diff --git a/openedx/features/content_tagging/apps.py b/openedx/features/content_tagging/apps.py new file mode 100644 index 000000000000..29f9c5005f43 --- /dev/null +++ b/openedx/features/content_tagging/apps.py @@ -0,0 +1,12 @@ +""" +Define the content tagging Django App. +""" + +from django.apps import AppConfig + + +class ContentTaggingConfig(AppConfig): + """App config for the content tagging feature""" + + default_auto_field = "django.db.models.BigAutoField" + name = "openedx.features.content_tagging" diff --git a/openedx/features/content_tagging/migrations/0001_initial.py b/openedx/features/content_tagging/migrations/0001_initial.py new file mode 100644 index 000000000000..1c6cc251c9e9 --- /dev/null +++ b/openedx/features/content_tagging/migrations/0001_initial.py @@ -0,0 +1,72 @@ +# Generated by Django 3.2.19 on 2023-06-19 22:44 + +from django.db import migrations, models +import django.db.models.deletion + + +class Migration(migrations.Migration): + initial = True + + dependencies = [ + ("organizations", "0003_historicalorganizationcourse"), + ("oel_tagging", "0001_initial"), + ] + + operations = [ + migrations.CreateModel( + name="ContentTaxonomy", + fields=[ + ( + "taxonomy_ptr", + models.OneToOneField( + auto_created=True, + on_delete=django.db.models.deletion.CASCADE, + parent_link=True, + primary_key=True, + serialize=False, + to="oel_tagging.taxonomy", + ), + ), + ], + options={ + "verbose_name_plural": "ContentTaxonomies", + }, + bases=("oel_tagging.taxonomy",), + ), + migrations.CreateModel( + name="ContentTaxonomyOrg", + fields=[ + ( + "id", + models.BigAutoField( + auto_created=True, + primary_key=True, + serialize=False, + verbose_name="ID", + ), + ), + ( + "org", + models.ForeignKey( + on_delete=django.db.models.deletion.CASCADE, + to="organizations.organization", + ), + ), + ( + "taxonomy", + models.ForeignKey( + on_delete=django.db.models.deletion.CASCADE, + to="content_tagging.contenttaxonomy", + ), + ), + ], + ), + migrations.AddField( + model_name="contenttaxonomy", + name="org_owners", + field=models.ManyToManyField( + through="content_tagging.ContentTaxonomyOrg", + to="organizations.Organization", + ), + ), + ] diff --git a/openedx/features/content_tagging/migrations/__init__.py b/openedx/features/content_tagging/migrations/__init__.py new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/openedx/features/content_tagging/models.py b/openedx/features/content_tagging/models.py new file mode 100644 index 000000000000..e5caacc94fc7 --- /dev/null +++ b/openedx/features/content_tagging/models.py @@ -0,0 +1,103 @@ +""" +Content Tagging models +""" +from django.db import models +from opaque_keys import InvalidKeyError +from opaque_keys.edx.keys import CourseKey, UsageKey +from openedx_tagging.core.tagging.models import Taxonomy, TaxonomyManager +from organizations.models import Organization + + +class ContentTaxonomyManager(TaxonomyManager): + """ + Manages ContentTaxonomy objects, providing custom utility methods. + + Inherits from InheritanceManager so that subclasses of ContentTaxonomy can be selected in a queryset. + """ + + def filter_enabled(self, org: Organization = None, enabled=True) -> models.QuerySet: + """ + Returns a query set filtered to return the enabled ContentTaxonomy objects owned by the given org. + """ + queryset = self + if enabled is not None: + queryset = queryset.filter(enabled=enabled) + org_filter = models.Q(org_owners=None) + if org: + org_filter |= models.Q(org_owners=org) + queryset = queryset.filter(org_filter) + return queryset + + +class ContentTaxonomy(Taxonomy): + """ + Taxonomy used for tagging content-related objects. + + ContentTaxonomies can be owned by one or more organizations, which allows content authors from that organization to + create taxonomies, edit the taxonomy fields, and add/edit/remove Tags linked to the taxonomy. + + .. no_pii: + """ + + objects = ContentTaxonomyManager() + + org_owners = models.ManyToManyField(Organization, through="ContentTaxonomyOrg") + + class Meta: + verbose_name_plural = "ContentTaxonomies" + + def validate_object_tag( + self, + object_tag: "ObjectTag", + check_taxonomy=True, + check_tag=True, + check_object=True, + ) -> bool: + """ + Returns True if the given object tag is valid for this ContentTaxonomy. + + Extends the superclass method by adding its own object checks to ensure: + + * object_tag.object_id is a valid UsageKey or CourseKey, and + * object_tag.object_id's "org" is enabled for this taxonomy. + """ + if check_object: + # ContentTaxonomies require object_id to be a valid CourseKey or UsageKey + try: + object_key = UsageKey.from_string(object_tag.object_id) + except InvalidKeyError: + try: + object_key = CourseKey.from_string(object_tag.object_id) + except InvalidKeyError: + return False + + # ...and object to be in an org that is enabled for this taxonomy. + if not self.enabled_for_org(object_key.org): + return False + + return super().validate_object_tag( + object_tag, + check_taxonomy=check_taxonomy, + check_tag=check_tag, + check_object=check_object, + ) + + def enabled_for_org(self, org_short_name: str) -> bool: + """ + Returns True if this taxonomy is enabled for the given organization. + """ + enabled = self.enabled + if self.org_owners.count(): + enabled &= self.org_owners.filter( + contenttaxonomyorg__org__short_name=org_short_name, + ).exists() + return enabled + + +class ContentTaxonomyOrg(models.Model): + """ + Represents the many-to-many relationship between ContentTaxonomies and Organizations. + """ + + taxonomy = models.ForeignKey(ContentTaxonomy, on_delete=models.CASCADE) + org = models.ForeignKey(Organization, on_delete=models.CASCADE) diff --git a/openedx/features/content_tagging/rules.py b/openedx/features/content_tagging/rules.py new file mode 100644 index 000000000000..46e65aa65703 --- /dev/null +++ b/openedx/features/content_tagging/rules.py @@ -0,0 +1,106 @@ +"""Django rules-based permissions for tagging""" + +import openedx_tagging.core.tagging.rules as oel_tagging +import rules +from django.contrib.auth import get_user_model +from organizations.models import Organization + +from common.djangoapps.student.auth import is_content_creator + +from .models import ContentTaxonomy + +User = get_user_model() + + +def is_taxonomy_admin(user: User, taxonomy: ContentTaxonomy = None) -> bool: + """ + Returns True if the given user is a Taxonomy Admin for the given content taxonomy. + + Global Taxonomy Admins include global staff and superusers, plus course creators who can create courses for any org. + + Otherwise, a taxonomy must be provided to determine if the user is a org-level course creator for one of the + taxonomy's org_owners. + """ + if oel_tagging.is_taxonomy_admin(user): + return True + + if not taxonomy: + return is_content_creator(user, None) + + # If the user is a content creator for any of this taxonomy's org_owners, they are also an admin for this taxonomy. + taxonomy_orgs = taxonomy.org_owners.values_list( + "contenttaxonomyorg__org__short_name", flat=True + ) + + # Fetch whole orgs list if the taxonomy doesn't specify org_owners + if not taxonomy_orgs: + taxonomy_orgs = Organization.objects.all().values_list("short_name", flat=True) + + if taxonomy_orgs: + for org in taxonomy_orgs: + if is_content_creator(user, org): + return True + return False + + +@rules.predicate +def can_view_taxonomy(user: User, taxonomy: ContentTaxonomy = None) -> bool: + """ + Anyone can view an enabled taxonomy, + but only taxonomy admins can view a disabled taxonomy. + """ + return (taxonomy and taxonomy.enabled) or is_taxonomy_admin(user, taxonomy) + + +@rules.predicate +def can_change_taxonomy(user: User, taxonomy: ContentTaxonomy = None) -> bool: + """ + Even taxonomy admins cannot change system taxonomies. + """ + return is_taxonomy_admin(user, taxonomy) and ( + not taxonomy or (taxonomy and not taxonomy.system_defined) + ) + + +@rules.predicate +def can_change_taxonomy_tag(user: User, tag: oel_tagging.Tag = None) -> bool: + """ + Even taxonomy admins cannot add tags to system taxonomies (their tags are system-defined), or free-text taxonomies + (these don't have predefined tags). + """ + taxonomy = tag.taxonomy if tag else None + return is_taxonomy_admin(user, taxonomy) and ( + not tag + or not taxonomy + or (taxonomy and not taxonomy.allow_free_text and not taxonomy.system_defined) + ) + + +@rules.predicate +def can_change_object_tag(user: User, object_tag: oel_tagging.ObjectTag = None) -> bool: + """ + Taxonomy admins can create or modify object tags on enabled taxonomies. + """ + taxonomy = object_tag.taxonomy if object_tag else None + return is_taxonomy_admin(user, taxonomy) and ( + not object_tag or not taxonomy or (taxonomy and taxonomy.enabled) + ) + + +# Taxonomy +rules.set_perm("oel_tagging.add_taxonomy", can_change_taxonomy) +rules.set_perm("oel_tagging.change_taxonomy", can_change_taxonomy) +rules.set_perm("oel_tagging.delete_taxonomy", can_change_taxonomy) +rules.set_perm("oel_tagging.view_taxonomy", can_view_taxonomy) + +# Tag +rules.set_perm("oel_tagging.add_tag", can_change_taxonomy_tag) +rules.set_perm("oel_tagging.change_tag", can_change_taxonomy_tag) +rules.set_perm("oel_tagging.delete_tag", can_change_taxonomy_tag) +rules.set_perm("oel_tagging.view_tag", rules.always_allow) + +# ObjectTag +rules.set_perm("oel_tagging.add_object_tag", can_change_object_tag) +rules.set_perm("oel_tagging.change_object_tag", can_change_object_tag) +rules.set_perm("oel_tagging.delete_object_tag", can_change_object_tag) +rules.set_perm("oel_tagging.view_object_tag", rules.always_allow) diff --git a/openedx/features/content_tagging/tests/__init__.py b/openedx/features/content_tagging/tests/__init__.py new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/openedx/features/content_tagging/tests/test_api.py b/openedx/features/content_tagging/tests/test_api.py new file mode 100644 index 000000000000..3b819c9b5bfa --- /dev/null +++ b/openedx/features/content_tagging/tests/test_api.py @@ -0,0 +1,251 @@ +"""Tests for the Tagging models""" +import ddt +from django.test.testcases import TestCase +from openedx_tagging.core.tagging.models import ObjectTag, Tag +from organizations.models import Organization + +from .. import api + + +class TestContentTaxonomyMixin: + """ + Sets up data for testing ContentTaxonomies. + """ + + def setUp(self): + super().setUp() + self.org1 = Organization.objects.create(name="OpenedX", short_name="OeX") + self.org2 = Organization.objects.create(name="Axim", short_name="Ax") + # Taxonomy + self.taxonomy_base = api.oel_tagging.create_taxonomy( + name="Base Taxonomy", + ) + # ContentTaxonomies + self.taxonomy_disabled = api.create_taxonomy( + name="Learning Objectives", + enabled=False, + ) + self.taxonomy_all_orgs = api.create_taxonomy( + name="Content Types", + enabled=True, + ) + self.taxonomy_both_orgs = api.create_taxonomy( + name="OpenedX/Axim Content Types", + enabled=True, + org_owners=[self.org1, self.org2], + ) + self.taxonomy_one_org = api.create_taxonomy( + name="OpenedX Content Types", + enabled=True, + org_owners=[self.org1], + ) + # Tags + self.tag_disabled = Tag.objects.create( + taxonomy=self.taxonomy_disabled, + value="learning", + ) + self.tag_all_orgs = Tag.objects.create( + taxonomy=self.taxonomy_all_orgs, + value="learning", + ) + self.tag_both_orgs = Tag.objects.create( + taxonomy=self.taxonomy_both_orgs, + value="learning", + ) + self.tag_one_org = Tag.objects.create( + taxonomy=self.taxonomy_one_org, + value="learning", + ) + # ObjectTags + self.all_orgs_course_tag = api.tag_object( + taxonomy=self.taxonomy_all_orgs, + tags=[self.tag_all_orgs.id], + object_id="course-v1:OeX+DemoX+Demo_Course", + object_type="course", + )[0] + self.all_orgs_block_tag = api.tag_object( + taxonomy=self.taxonomy_all_orgs, + tags=[self.tag_all_orgs.id], + object_id="block-v1:Ax+DemoX+Demo_Course+type@vertical+block@abcde", + object_type="section", + )[0] + self.both_orgs_course_tag = api.tag_object( + taxonomy=self.taxonomy_both_orgs, + tags=[self.tag_both_orgs.id], + object_id="course-v1:Ax+DemoX+Demo_Course", + object_type="course", + )[0] + self.both_orgs_block_tag = api.tag_object( + taxonomy=self.taxonomy_both_orgs, + tags=[self.tag_both_orgs.id], + object_id="block-v1:OeX+DemoX+Demo_Course+type@vertical+block@abcde", + object_type="section", + )[0] + self.one_org_block_tag = api.tag_object( + taxonomy=self.taxonomy_one_org, + tags=[self.tag_one_org.id], + object_id="block-v1:OeX+DemoX+Demo_Course+type@vertical+block@abcde", + object_type="section", + )[0] + + # Invalid object tags must be manually created + self.disabled_course_tag = ObjectTag.objects.create( + taxonomy=self.taxonomy_disabled, + tag=self.tag_disabled, + object_id="course-v1:Ax+DemoX+Demo_Course", + object_type="course", + ) + self.all_orgs_invalid_tag = ObjectTag.objects.create( + taxonomy=self.taxonomy_all_orgs, + tag=self.tag_all_orgs, + object_id="course-v1_OpenedX_DemoX_Demo_Course", + object_type="course", + ) + self.one_org_invalid_org_tag = ObjectTag.objects.create( + taxonomy=self.taxonomy_one_org, + tag=self.tag_one_org, + object_id="course-v1:Ax+DemoX+Demo_Course", + object_type="course", + ) + + +@ddt.ddt +class TestAPIContentTaxonomy(TestContentTaxonomyMixin, TestCase): + """ + Tests the ContentTaxonomy APIs. + """ + + def test_get_taxonomies_enabled_subclasses(self): + with self.assertNumQueries(1): + taxonomies = list(api.get_taxonomies()) + assert taxonomies == [ + self.taxonomy_base, + self.taxonomy_all_orgs, + self.taxonomy_one_org, + self.taxonomy_both_orgs, + ] + + @ddt.data( + # All orgs + (None, True, ["taxonomy_all_orgs"]), + (None, False, ["taxonomy_disabled"]), + (None, None, ["taxonomy_all_orgs", "taxonomy_disabled"]), + # Org 1 + ("org1", True, ["taxonomy_all_orgs", "taxonomy_one_org", "taxonomy_both_orgs"]), + ("org1", False, ["taxonomy_disabled"]), + ( + "org1", + None, + [ + "taxonomy_all_orgs", + "taxonomy_disabled", + "taxonomy_one_org", + "taxonomy_both_orgs", + ], + ), + # Org 2 + ("org2", True, ["taxonomy_all_orgs", "taxonomy_both_orgs"]), + ("org2", False, ["taxonomy_disabled"]), + ( + "org2", + None, + ["taxonomy_all_orgs", "taxonomy_disabled", "taxonomy_both_orgs"], + ), + ) + @ddt.unpack + def test_get_taxonomies_for_org(self, org_attr, enabled, expected): + org_owner = getattr(self, org_attr) if org_attr else None + with self.assertNumQueries(1): + taxonomies = list( + api.get_taxonomies_for_org(org_owner=org_owner, enabled=enabled) + ) + assert taxonomies == [ + getattr(self, taxonomy_attr) for taxonomy_attr in expected + ] + + @ddt.data( + ("taxonomy_all_orgs", "all_orgs_course_tag", 2), + ("taxonomy_all_orgs", "all_orgs_block_tag", 2), + ("taxonomy_both_orgs", "both_orgs_course_tag", 3), + ("taxonomy_both_orgs", "both_orgs_block_tag", 3), + ("taxonomy_one_org", "one_org_block_tag", 3), + ) + @ddt.unpack + def test_get_object_tags_valid_for_org( + self, taxonomy_attr, object_tag_attr, num_queries + ): + taxonomy_id = getattr(self, taxonomy_attr).id + taxonomy = api.get_taxonomy(taxonomy_id) + object_tag = getattr(self, object_tag_attr) + with self.assertNumQueries(num_queries): + valid_tags = api.get_object_tags( + taxonomy, + object_tag.object_id, + object_tag.object_type, + valid_only=True, + ) + assert valid_tags == [object_tag] + + @ddt.data( + ("taxonomy_disabled", "disabled_course_tag", 1), + ("taxonomy_all_orgs", "all_orgs_course_tag", 1), + ("taxonomy_all_orgs", "all_orgs_block_tag", 1), + ("taxonomy_all_orgs", "all_orgs_invalid_tag", 1), + ("taxonomy_both_orgs", "both_orgs_course_tag", 1), + ("taxonomy_both_orgs", "both_orgs_block_tag", 1), + ("taxonomy_one_org", "one_org_block_tag", 1), + ("taxonomy_one_org", "one_org_invalid_org_tag", 1), + ) + @ddt.unpack + def test_get_object_tags_include_invalid( + self, taxonomy_attr, object_tag_attr, num_queries + ): + taxonomy_id = getattr(self, taxonomy_attr).id + taxonomy = api.get_taxonomy(taxonomy_id) + object_tag = getattr(self, object_tag_attr) + with self.assertNumQueries(num_queries): + valid_tags = api.get_object_tags( + taxonomy, + object_tag.object_id, + object_tag.object_type, + valid_only=False, + ) + assert valid_tags == [object_tag] + + @ddt.data( + "all_orgs_invalid_tag", + "one_org_invalid_org_tag", + ) + def test_validate_object_tag_check_object(self, tag_attr): + """ + Check that the subclass is correctly checking the object part of the ObjectTag. + """ + object_tag = getattr(self, tag_attr) + assert not object_tag.is_valid + taxonomy = api.get_taxonomy(object_tag.taxonomy_id) + assert not taxonomy.validate_object_tag( + object_tag, check_object=True, check_taxonomy=False, check_tag=False + ) + assert taxonomy.validate_object_tag( + object_tag, check_object=False, check_taxonomy=True, check_tag=True + ) + + def test_get_tags(self): + assert api.get_tags(self.taxonomy_all_orgs) == [self.tag_all_orgs] + + def test_resync_object_tags(self): + # Only all_orgs_invalid_tag needs re-syncing, because it wasn't created with api.tag_object + assert ( + api.resync_object_tags( + ObjectTag.objects.filter(taxonomy=self.taxonomy_all_orgs) + ) + == 1 + ) + self.taxonomy_all_orgs.delete() + for object_tag in [ + self.all_orgs_course_tag, + self.all_orgs_block_tag, + self.all_orgs_invalid_tag, + ]: + object_tag.refresh_from_db() + assert object_tag.name == "Content Types" diff --git a/openedx/features/content_tagging/tests/test_rules.py b/openedx/features/content_tagging/tests/test_rules.py new file mode 100644 index 000000000000..e8460307a0ff --- /dev/null +++ b/openedx/features/content_tagging/tests/test_rules.py @@ -0,0 +1,319 @@ +"""Tests content_tagging rules-based permissions""" + +import ddt +from django.contrib.auth import get_user_model +from django.test.testcases import TestCase, override_settings +from mock import Mock +from openedx_tagging.core.tagging.models import ObjectTag, Tag + +from common.djangoapps.student.auth import add_users, update_org_role +from common.djangoapps.student.roles import CourseCreatorRole, OrgContentCreatorRole + +from ..models import ContentTaxonomy +from .test_api import TestContentTaxonomyMixin + +User = get_user_model() + + +@ddt.ddt +@override_settings(FEATURES={"ENABLE_CREATOR_GROUP": True}) +class TestRulesContentTaxonomy(TestContentTaxonomyMixin, TestCase): + """ + Tests that the expected rules have been applied to the ContentTaxonomy models. + + We set ENABLE_CREATOR_GROUP for these tests, otherwise all users have course creator access for all orgs. + """ + + def setUp(self): + super().setUp() + self.superuser = User.objects.create( + username="superuser", + email="superuser@example.com", + is_superuser=True, + ) + self.staff = User.objects.create( + username="staff", + email="staff@example.com", + is_staff=True, + ) + # Normal user: grant course creator role (for all orgs) + self.user_all_orgs = User.objects.create( + username="user_all_orgs", + email="staff+all@example.com", + ) + add_users(self.staff, CourseCreatorRole(), self.user_all_orgs) + + # Normal user: grant course creator access to both org1 and org2 + self.user_both_orgs = User.objects.create( + username="user_both_orgs", + email="staff+both@example.com", + ) + update_org_role( + self.staff, + OrgContentCreatorRole, + self.user_both_orgs, + [self.org1.short_name, self.org2.short_name], + ) + + # Normal user: grant course creator access to org2 + self.user_org2 = User.objects.create( + username="user_org2", + email="staff+org2@example.com", + ) + update_org_role( + self.staff, OrgContentCreatorRole, self.user_org2, [self.org2.short_name] + ) + + # Normal user: no course creator access + self.learner = User.objects.create( + username="learner", + email="learner@example.com", + ) + + def _expected_users_have_perm( + self, perm, obj, learner_perm=False, learner_obj=False, user_org2=True + ): + """ + Checks that all users have the given permission on the given object. + + If learners_too, then the learner user should have it too. + """ + # Global Taxonomy Admins can do pretty much anything + assert self.superuser.has_perm(perm) + assert self.superuser.has_perm(perm, obj) + assert self.staff.has_perm(perm) + assert self.staff.has_perm(perm, obj) + assert self.user_all_orgs.has_perm(perm) + assert self.user_all_orgs.has_perm(perm, obj) + + # Org content creators are bound by a taxonomy's org restrictions + assert self.user_both_orgs.has_perm(perm) == learner_perm + assert self.user_both_orgs.has_perm(perm, obj) + assert self.user_org2.has_perm(perm) == learner_perm + # user_org2 does not have course creator access for org 1 + assert self.user_org2.has_perm(perm, obj) == user_org2 + + # Learners can't do much but view + assert self.learner.has_perm(perm) == learner_perm + assert self.learner.has_perm(perm, obj) == learner_obj + + # Taxonomy + + @ddt.data( + ("oel_tagging.add_taxonomy", "taxonomy_all_orgs"), + ("oel_tagging.add_taxonomy", "taxonomy_both_orgs"), + ("oel_tagging.add_taxonomy", "taxonomy_disabled"), + ("oel_tagging.change_taxonomy", "taxonomy_all_orgs"), + ("oel_tagging.change_taxonomy", "taxonomy_both_orgs"), + ("oel_tagging.change_taxonomy", "taxonomy_disabled"), + ("oel_tagging.delete_taxonomy", "taxonomy_all_orgs"), + ("oel_tagging.delete_taxonomy", "taxonomy_both_orgs"), + ("oel_tagging.delete_taxonomy", "taxonomy_disabled"), + ) + @ddt.unpack + def test_change_taxonomy_all_orgs(self, perm, taxonomy_attr): + """Taxonomy administrators with course creator access for the taxonomy org""" + taxonomy = getattr(self, taxonomy_attr) + self._expected_users_have_perm(perm, taxonomy) + + @ddt.data( + ("oel_tagging.add_taxonomy", "taxonomy_one_org"), + ("oel_tagging.change_taxonomy", "taxonomy_one_org"), + ("oel_tagging.delete_taxonomy", "taxonomy_one_org"), + ) + @ddt.unpack + def test_change_taxonomy_org1(self, perm, taxonomy_attr): + taxonomy = getattr(self, taxonomy_attr) + self._expected_users_have_perm(perm, taxonomy, user_org2=False) + + @ddt.data( + "oel_tagging.add_taxonomy", + "oel_tagging.change_taxonomy", + "oel_tagging.delete_taxonomy", + ) + def test_system_taxonomy(self, perm): + """Taxonomy administrators cannot edit system taxonomies""" + # TODO: use SystemTaxonomy when available + system_taxonomy = Mock(spec=ContentTaxonomy) + system_taxonomy.system_defined.return_value = True + system_taxonomy.org_owners.return_value = Mock() + system_taxonomy.org_owners.values_list.return_value = [] + assert self.superuser.has_perm(perm, system_taxonomy) + assert not self.staff.has_perm(perm, system_taxonomy) + assert not self.user_all_orgs.has_perm(perm, system_taxonomy) + assert not self.user_both_orgs.has_perm(perm, system_taxonomy) + assert not self.user_org2.has_perm(perm, system_taxonomy) + assert not self.learner.has_perm(perm, system_taxonomy) + + @ddt.data( + (True, "taxonomy_all_orgs"), + (False, "taxonomy_all_orgs"), + (True, "taxonomy_both_orgs"), + (False, "taxonomy_both_orgs"), + ) + @ddt.unpack + def test_view_taxonomy_enabled(self, enabled, taxonomy_attr): + """Anyone can see enabled taxonomies, but learners cannot see disabled taxonomies""" + taxonomy = getattr(self, taxonomy_attr) + taxonomy.enabled = enabled + perm = "oel_tagging.view_taxonomy" + self._expected_users_have_perm(perm, taxonomy, learner_obj=enabled) + + # Tag + + @ddt.data( + ("oel_tagging.add_tag", "tag_all_orgs"), + ("oel_tagging.add_tag", "tag_both_orgs"), + ("oel_tagging.add_tag", "tag_disabled"), + ("oel_tagging.change_tag", "tag_all_orgs"), + ("oel_tagging.change_tag", "tag_both_orgs"), + ("oel_tagging.change_tag", "tag_disabled"), + ("oel_tagging.delete_tag", "tag_all_orgs"), + ("oel_tagging.delete_tag", "tag_both_orgs"), + ("oel_tagging.delete_tag", "tag_disabled"), + ) + @ddt.unpack + def test_change_tag_all_orgs(self, perm, tag_attr): + """Taxonomy administrators can modify tags on non-free-text taxonomies""" + tag = getattr(self, tag_attr) + self._expected_users_have_perm(perm, tag) + + @ddt.data( + ("oel_tagging.add_tag", "tag_one_org"), + ("oel_tagging.change_tag", "tag_one_org"), + ("oel_tagging.delete_tag", "tag_one_org"), + ) + @ddt.unpack + def test_change_tag_org1(self, perm, tag_attr): + """Taxonomy administrators can modify tags on non-free-text taxonomies""" + tag = getattr(self, tag_attr) + self._expected_users_have_perm(perm, tag, user_org2=False) + + @ddt.data( + "oel_tagging.add_tag", + "oel_tagging.change_tag", + "oel_tagging.delete_tag", + ) + def test_tag_no_taxonomy(self, perm): + """Taxonomy administrators can modify any Tag, even those with no Taxonnmy.""" + tag = Tag() + + # Global Taxonomy Admins can do pretty much anything + assert self.superuser.has_perm(perm, tag) + assert self.staff.has_perm(perm, tag) + assert self.user_all_orgs.has_perm(perm, tag) + + # Org content creators are bound by a taxonomy's org restrictions, + # so if there's no taxonomy, they can't do anything to it. + assert not self.user_both_orgs.has_perm(perm, tag) + assert not self.user_org2.has_perm(perm, tag) + assert not self.learner.has_perm(perm, tag) + + @ddt.data( + "tag_all_orgs", + "tag_both_orgs", + "tag_one_org", + "tag_disabled", + ) + def test_view_tag(self, tag_attr): + """Anyone can view any Tag""" + tag = getattr(self, tag_attr) + self._expected_users_have_perm( + "oel_tagging.view_tag", tag, learner_perm=True, learner_obj=True + ) + + # ObjectTag + + @ddt.data( + ("oel_tagging.add_object_tag", "disabled_course_tag"), + ("oel_tagging.change_object_tag", "disabled_course_tag"), + ("oel_tagging.delete_object_tag", "disabled_course_tag"), + ) + @ddt.unpack + def test_object_tag_disabled_taxonomy(self, perm, tag_attr): + """Taxonomy administrators cannot create/edit an ObjectTag with a disabled Taxonomy""" + object_tag = getattr(self, tag_attr) + assert not object_tag.taxonomy.enabled + assert self.superuser.has_perm(perm, object_tag) + assert not self.staff.has_perm(perm, object_tag) + assert not self.user_all_orgs.has_perm(perm, object_tag) + assert not self.user_both_orgs.has_perm(perm, object_tag) + assert not self.user_org2.has_perm(perm, object_tag) + assert not self.learner.has_perm(perm, object_tag) + + @ddt.data( + ("oel_tagging.add_object_tag", "all_orgs_course_tag"), + ("oel_tagging.add_object_tag", "all_orgs_block_tag"), + ("oel_tagging.add_object_tag", "both_orgs_course_tag"), + ("oel_tagging.add_object_tag", "both_orgs_block_tag"), + ("oel_tagging.add_object_tag", "all_orgs_invalid_tag"), + ("oel_tagging.change_object_tag", "all_orgs_course_tag"), + ("oel_tagging.change_object_tag", "all_orgs_block_tag"), + ("oel_tagging.change_object_tag", "both_orgs_course_tag"), + ("oel_tagging.change_object_tag", "both_orgs_block_tag"), + ("oel_tagging.change_object_tag", "all_orgs_invalid_tag"), + ("oel_tagging.delete_object_tag", "all_orgs_course_tag"), + ("oel_tagging.delete_object_tag", "all_orgs_block_tag"), + ("oel_tagging.delete_object_tag", "both_orgs_course_tag"), + ("oel_tagging.delete_object_tag", "both_orgs_block_tag"), + ("oel_tagging.delete_object_tag", "all_orgs_invalid_tag"), + ) + @ddt.unpack + def test_change_object_tag_all_orgs(self, perm, tag_attr): + """Taxonomy administrators can create/edit an ObjectTag on taxonomies in their org.""" + object_tag = getattr(self, tag_attr) + self._expected_users_have_perm(perm, object_tag) + + @ddt.data( + ("oel_tagging.add_object_tag", "one_org_block_tag"), + ("oel_tagging.add_object_tag", "one_org_invalid_org_tag"), + ("oel_tagging.change_object_tag", "one_org_block_tag"), + ("oel_tagging.change_object_tag", "one_org_invalid_org_tag"), + ("oel_tagging.delete_object_tag", "one_org_block_tag"), + ("oel_tagging.delete_object_tag", "one_org_invalid_org_tag"), + ) + @ddt.unpack + def test_change_object_tag_org1(self, perm, tag_attr): + """Taxonomy administrators can create/edit an ObjectTag on taxonomies in their org.""" + object_tag = getattr(self, tag_attr) + self._expected_users_have_perm(perm, object_tag, user_org2=False) + + @ddt.data( + "oel_tagging.add_object_tag", + "oel_tagging.change_object_tag", + "oel_tagging.delete_object_tag", + ) + def test_object_tag_no_taxonomy(self, perm): + """Taxonomy administrators can modify an ObjectTag with no Taxonomy""" + object_tag = ObjectTag() + + # Global Taxonomy Admins can do pretty much anything + assert self.superuser.has_perm(perm, object_tag) + assert self.staff.has_perm(perm, object_tag) + assert self.user_all_orgs.has_perm(perm, object_tag) + + # Org content creators are bound by a taxonomy's org restrictions, + # so if there's no taxonomy, they can't do anything to it. + assert not self.user_both_orgs.has_perm(perm, object_tag) + assert not self.user_org2.has_perm(perm, object_tag) + assert not self.learner.has_perm(perm, object_tag) + + @ddt.data( + "all_orgs_course_tag", + "all_orgs_block_tag", + "both_orgs_course_tag", + "both_orgs_block_tag", + "one_org_block_tag", + "all_orgs_invalid_tag", + "one_org_invalid_org_tag", + "disabled_course_tag", + ) + def test_view_object_tag(self, tag_attr): + """Anyone can view any ObjectTag""" + object_tag = getattr(self, tag_attr) + self._expected_users_have_perm( + "oel_tagging.view_object_tag", + object_tag, + learner_perm=True, + learner_obj=True, + ) diff --git a/requirements/edx/base.txt b/requirements/edx/base.txt index ce5cf2b3923e..242d5e903408 100644 --- a/requirements/edx/base.txt +++ b/requirements/edx/base.txt @@ -241,6 +241,7 @@ django==3.2.19 # openedx-django-wiki # openedx-events # openedx-filters + # openedx-learning # ora2 # outcome-surveys # skill-tagging @@ -402,6 +403,7 @@ djangorestframework==3.13.1 # edx-submissions # learner-pathway-progress # openedx-blockstore + # openedx-learning # ora2 # super-csv djangorestframework-xml==2.0.0 @@ -792,6 +794,8 @@ openedx-filters==1.3.0 # -r requirements/edx/base.in # lti-consumer-xblock # skill-tagging +openedx-learning @ git+https://github.com/open-craft/openedx-learning.git@jill/taxonomy-inheritance-fix + # via -r requirements/edx/github.in openedx-mongodbproxy==0.2.0 # via -r requirements/edx/base.in optimizely-sdk==4.1.1 @@ -1029,6 +1033,7 @@ rules==3.3 # -r requirements/edx/base.in # edx-enterprise # edx-proctoring + # openedx-learning s3transfer==0.1.13 # via boto3 sailthru-client==2.2.3 diff --git a/requirements/edx/development.txt b/requirements/edx/development.txt index cfb3f460a115..3350c0c31258 100644 --- a/requirements/edx/development.txt +++ b/requirements/edx/development.txt @@ -342,6 +342,7 @@ django==3.2.19 # openedx-django-wiki # openedx-events # openedx-filters + # openedx-learning # ora2 # outcome-surveys # skill-tagging @@ -517,6 +518,7 @@ djangorestframework==3.13.1 # edx-submissions # learner-pathway-progress # openedx-blockstore + # openedx-learning # ora2 # super-csv djangorestframework-xml==2.0.0 @@ -1052,6 +1054,8 @@ openedx-filters==1.3.0 # -r requirements/edx/testing.txt # lti-consumer-xblock # skill-tagging +openedx-learning @ git+https://github.com/open-craft/openedx-learning.git@jill/taxonomy-inheritance-fix + # via -r requirements/edx/testing.txt openedx-mongodbproxy==0.2.0 # via -r requirements/edx/testing.txt optimizely-sdk==4.1.1 @@ -1436,6 +1440,7 @@ rules==3.3 # -r requirements/edx/testing.txt # edx-enterprise # edx-proctoring + # openedx-learning s3transfer==0.1.13 # via # -r requirements/edx/testing.txt diff --git a/requirements/edx/github.in b/requirements/edx/github.in index 2b4e75f5222c..a35a9b39e857 100644 --- a/requirements/edx/github.in +++ b/requirements/edx/github.in @@ -79,6 +79,8 @@ ############################################################################## # ... add dependencies here +# FIXME JV - move to base.in once pypi package created +git+https://github.com/open-craft/openedx-learning.git@jill/taxonomy-inheritance-fix#egg=openedx-learning==0.1 ############################################################################## diff --git a/requirements/edx/testing.txt b/requirements/edx/testing.txt index 0ddfb8864232..04066fedb717 100644 --- a/requirements/edx/testing.txt +++ b/requirements/edx/testing.txt @@ -320,6 +320,7 @@ django==3.2.19 # openedx-django-wiki # openedx-events # openedx-filters + # openedx-learning # ora2 # outcome-surveys # skill-tagging @@ -493,6 +494,7 @@ djangorestframework==3.13.1 # edx-submissions # learner-pathway-progress # openedx-blockstore + # openedx-learning # ora2 # super-csv djangorestframework-xml==2.0.0 @@ -997,6 +999,8 @@ openedx-filters==1.3.0 # -r requirements/edx/base.txt # lti-consumer-xblock # skill-tagging +openedx-learning @ git+https://github.com/open-craft/openedx-learning.git@jill/taxonomy-inheritance-fix + # via -r requirements/edx/base.txt openedx-mongodbproxy==0.2.0 # via -r requirements/edx/base.txt optimizely-sdk==4.1.1 @@ -1348,6 +1352,7 @@ rules==3.3 # -r requirements/edx/base.txt # edx-enterprise # edx-proctoring + # openedx-learning s3transfer==0.1.13 # via # -r requirements/edx/base.txt