From fec6472d166e8f216203557c772b7de67e73faad Mon Sep 17 00:00:00 2001 From: XnpioChV Date: Sun, 28 May 2023 17:04:11 -0500 Subject: [PATCH 1/9] feat: Tagging app created --- .gitignore | 1 + openedx_learning/contrib/tagging/__init__.py | 0 openedx_learning/contrib/tagging/admin.py | 5 +++ openedx_learning/contrib/tagging/apps.py | 16 ++++++++ .../tagging/migrations/0001_initial.py | 23 +++++++++++ .../contrib/tagging/migrations/__init__.py | 0 openedx_learning/contrib/tagging/models.py | 38 +++++++++++++++++++ openedx_learning/contrib/tagging/readme.rst | 26 +++++++++++++ projects/dev.py | 1 + 9 files changed, 110 insertions(+) create mode 100644 openedx_learning/contrib/tagging/__init__.py create mode 100644 openedx_learning/contrib/tagging/admin.py create mode 100644 openedx_learning/contrib/tagging/apps.py create mode 100644 openedx_learning/contrib/tagging/migrations/0001_initial.py create mode 100644 openedx_learning/contrib/tagging/migrations/__init__.py create mode 100644 openedx_learning/contrib/tagging/models.py create mode 100644 openedx_learning/contrib/tagging/readme.rst diff --git a/.gitignore b/.gitignore index 888019d47..0fc03c7df 100644 --- a/.gitignore +++ b/.gitignore @@ -64,6 +64,7 @@ requirements/private.txt dev.db* .vscode +venv/ # Media files (for uploads) media/ diff --git a/openedx_learning/contrib/tagging/__init__.py b/openedx_learning/contrib/tagging/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/openedx_learning/contrib/tagging/admin.py b/openedx_learning/contrib/tagging/admin.py new file mode 100644 index 000000000..2e59ec267 --- /dev/null +++ b/openedx_learning/contrib/tagging/admin.py @@ -0,0 +1,5 @@ +""" Tagging app admin """ +from django.contrib import admin +from .models import TagContent + +admin.site.register(TagContent) diff --git a/openedx_learning/contrib/tagging/apps.py b/openedx_learning/contrib/tagging/apps.py new file mode 100644 index 000000000..46f97d066 --- /dev/null +++ b/openedx_learning/contrib/tagging/apps.py @@ -0,0 +1,16 @@ +""" +tagging Django application initialization. +""" + +from django.apps import AppConfig + + +class TaggingConfig(AppConfig): + """ + Configuration for the tagging Django application. + """ + + name = "openedx_learning.contrib.tagging" + verbose_name = "Tagging" + default_auto_field = 'django.db.models.BigAutoField' + label = "oel_tagging" diff --git a/openedx_learning/contrib/tagging/migrations/0001_initial.py b/openedx_learning/contrib/tagging/migrations/0001_initial.py new file mode 100644 index 000000000..f3b336ef3 --- /dev/null +++ b/openedx_learning/contrib/tagging/migrations/0001_initial.py @@ -0,0 +1,23 @@ +# Generated by Django 3.2.19 on 2023-05-25 21:08 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + initial = True + + dependencies = [ + ] + + operations = [ + migrations.CreateModel( + name='TagContent', + fields=[ + ('id', models.BigAutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), + ('content_id', models.CharField(max_length=255)), + ('name', models.CharField(max_length=255)), + ('value', models.CharField(max_length=765)), + ], + ), + ] diff --git a/openedx_learning/contrib/tagging/migrations/__init__.py b/openedx_learning/contrib/tagging/migrations/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/openedx_learning/contrib/tagging/models.py b/openedx_learning/contrib/tagging/models.py new file mode 100644 index 000000000..9f3a30d6f --- /dev/null +++ b/openedx_learning/contrib/tagging/models.py @@ -0,0 +1,38 @@ +""" Tagging app data models """ +from django.db import models + +class TagContent(models.Model): + """ + TODO: This is a basic representation of TagContent, it may change + depending on the discovery in: + https://docs.google.com/document/d/13zfsGDfomSTCp_G-_ncevQHAb4Y4UW0d_6N8R2PdHlA/edit#heading=h.o6fm1hktwp7b + + This is the representation of a tag that is linked to a content + + Name-Value pair + ----------------- + + We use a Field-Value Pair representation, where `name` functions + as the constant that defines the field data set, and `value` functions + as the variable tag lineage that belong to the set. + + Value lineage + --------------- + + `value` are stored as null character-delimited strings to preserve the tag's lineage. + We use the null character to avoid choosing a character that may exist in a tag's name. + We don't use the Taxonomy's user-facing delimiter so that this delimiter can be changed + without disrupting stored tags. + """ + + # Content id to which this tag is associated + content_id = models.CharField(max_length=255) + + # It's not usually large + name = models.CharField(max_length=255) + + # Tag lineage value. + # + # The lineage can be large. + # TODO: The length is under discussion + value = models.CharField(max_length=765) diff --git a/openedx_learning/contrib/tagging/readme.rst b/openedx_learning/contrib/tagging/readme.rst new file mode 100644 index 000000000..611269549 --- /dev/null +++ b/openedx_learning/contrib/tagging/readme.rst @@ -0,0 +1,26 @@ +Tagging App +============== + +The ``tagging`` app will enable content authors to tag pieces of content and quickly +filter for ease of re-use. + +Motivation +---------- + +Tagging content is central to enable content re-use, facilitate the implementation +of flexible content structures different from the current implementation and +allow adaptive learning in the Open edX platform. + +This service has been implemented as pluggable django app. Since it is necessary for +it to work independently of the content to which it links to. + +Setup +--------- + +**TODO:** We need to wait the discussion of the [Taxonomy discovery](https://docs.google.com/document/d/13zfsGDfomSTCp_G-_ncevQHAb4Y4UW0d_6N8R2PdHlA/edit#heading=h.o6fm1hktwp7b) +to build a proper setup for linking different pieces of content. + +The current approach is to save the id of the content through a generic string, +so that the tag can be linked to any type of content, no matter what type of ID have. +For example, with this approach we can link standalone blocks and library blocks, +both on Denver (v3) and Blockstore Content Libraries (v2). diff --git a/projects/dev.py b/projects/dev.py index 7be663f44..2d50a6119 100644 --- a/projects/dev.py +++ b/projects/dev.py @@ -35,6 +35,7 @@ "openedx_learning.core.publishing.apps.PublishingConfig", # Learning Contrib Apps "openedx_learning.contrib.media_server.apps.MediaServerConfig", + "openedx_learning.contrib.tagging.apps.TaggingConfig", # Apps that don't belong in this repo in the long term, but are here to make # testing/iteration easier until the APIs stabilize. "olx_importer.apps.OLXImporterConfig", From c3446a5d8c0e7760c3c704831f9d03b8c6ff2328 Mon Sep 17 00:00:00 2001 From: XnpioChV Date: Mon, 29 May 2023 16:03:10 -0500 Subject: [PATCH 2/9] docs: Nit on tagging app readme --- openedx_learning/contrib/tagging/readme.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/openedx_learning/contrib/tagging/readme.rst b/openedx_learning/contrib/tagging/readme.rst index 611269549..5ce22340d 100644 --- a/openedx_learning/contrib/tagging/readme.rst +++ b/openedx_learning/contrib/tagging/readme.rst @@ -17,7 +17,7 @@ it to work independently of the content to which it links to. Setup --------- -**TODO:** We need to wait the discussion of the [Taxonomy discovery](https://docs.google.com/document/d/13zfsGDfomSTCp_G-_ncevQHAb4Y4UW0d_6N8R2PdHlA/edit#heading=h.o6fm1hktwp7b) +**TODO:** We need to wait the discussion of the `Taxonomy discovery `_. to build a proper setup for linking different pieces of content. The current approach is to save the id of the content through a generic string, From c5fae57cd6322013adb8896a839b99bf4b4163cc Mon Sep 17 00:00:00 2001 From: Jillian Vogel Date: Thu, 1 Jun 2023 15:32:33 +0930 Subject: [PATCH 3/9] docs: adds ADRs arising from the taxonomy data architecture decisions --- docs/decisions/0007-tagging-app.rst | 21 +++++++ .../decisions/0008-tagging-tree-data-arch.rst | 53 ++++++++++++++++++ .../decisions/0009-tagging-administrators.rst | 37 ++++++++++++ .../0010-taxonomy-enable-context.rst | 56 +++++++++++++++++++ docs/decisions/0011-tag-changes.rst | 35 ++++++++++++ 5 files changed, 202 insertions(+) create mode 100644 docs/decisions/0007-tagging-app.rst create mode 100644 docs/decisions/0008-tagging-tree-data-arch.rst create mode 100644 docs/decisions/0009-tagging-administrators.rst create mode 100644 docs/decisions/0010-taxonomy-enable-context.rst create mode 100644 docs/decisions/0011-tag-changes.rst diff --git a/docs/decisions/0007-tagging-app.rst b/docs/decisions/0007-tagging-app.rst new file mode 100644 index 000000000..fca6349b4 --- /dev/null +++ b/docs/decisions/0007-tagging-app.rst @@ -0,0 +1,21 @@ +7. Tagging App structure +======================== + +Context +------- + +We want the openedx_tagging app to be useful in different Django projects outside of just openedx-learning and edx-platform. + + +Decisions +--------- + +The openedx_tagging data structures and code will stand alone with no dependencies on other Open edX projects. + +Classes which require dependencies on other Open edX projects should be defined within a ``tagging`` module inside those projects. + +For example, here we define ``openedx_tagging.core.models.Taxonomy``, whose data and functionality are self-contained to the openedx_tagging app. However in Studio, we need to be able to limit access to some Taxonomy by organization, using the same "course creator" access which limits course creation for an organization to a defined set of users. + +So in edx-platform, we create the ``cms.djangoapps.tagging`` app, and it contains ``models.OrgTaxonomy``, which has a 1:1 relationship with an ``openedx_tagging.core.models.Taxonomy``, so that ``openedx_tagging.core.models.Tag`` entries can be created which link to that Taxonomy. + +To support the org restrictions, `OrgTaxonomy` adds the ``org_owner`` field, a foreign key to the Organization model, which is provided by the edx-organization app. diff --git a/docs/decisions/0008-tagging-tree-data-arch.rst b/docs/decisions/0008-tagging-tree-data-arch.rst new file mode 100644 index 000000000..f4a879fef --- /dev/null +++ b/docs/decisions/0008-tagging-tree-data-arch.rst @@ -0,0 +1,53 @@ +8. Tag tree data structure +========================== + +Context +------- + +A taxonomy groups a set of related tags under a namespace and a set of usage rules. Some taxonomies require tags to be selected from a list or nested tree of valid options. + +Do we need a formal tree data structure to represent hierarchical tags? + +Decision +-------- + +No, a simplistic tree structure is sufficient for the MVP and forseeable feature requests. + +Existing tree data structures are designed to support very dynamic and deeply nested trees (e.g. forum threads) which are traversed frequently, and this feature set is overkill for taxonomy trees. + +Taxonomy trees have a maximum depth of 3 levels, which limits the depth of node traversal, and simplifies the UI/UX required to tag or search filter content with nested tags. + +Taxonomy trees only require simple operations, and infrequent traversals. Frequent operations (like viewing content tags) will only display the leaf tag value, not its full lineage, to minimize tree traversal. Full trees can be fetched quickly enough during content tag editing. Taxonomy tree changes themselves will also be infrequent. + +Rejected Alternatives +--------------------- + +All taxonomies are trees +~~~~~~~~~~~~~~~~~~~~~~~~ + +We could use a tree structure for all taxonomies: flat taxonomies would have only 1 level of tags under the root, while nested taxonomies can be deeper. + +To implement this, we'd link each taxonomy to a root tag, with the user-visible tags underneath. + +It was simpler instead to link the tag to the taxonomy, which removes the need for the unseen root tag. + +Closure tables +~~~~~~~~~~~~~~ + +https://coderwall.com/p/lixing/closure-tables-for-browsing-trees-in-sql + +Implementing the taxonomy tree using closure tables allows for tree traversals in O(N) time or less, where N is the total number of tags in the taxonomy. So the tree depth isn't as much of a performance concern as the total number of tags. + +Options include: + +* `django-tree-queries `_ + + Simple, performant, and well-maintained tree data structure. However it uses RECURSIVE CTE queries, which aren't supported until MySQL 8.0. + +* `django-mptt `_ + + Already an edx-platform dependency, but no longer maintained. It can be added retroactively to an existing tree-like model. + +* `django-closuretree `_ + + Another a good reference implementation for closure tables which can be added retroactively to an existing tree-like model. It is not actively maintained. diff --git a/docs/decisions/0009-tagging-administrators.rst b/docs/decisions/0009-tagging-administrators.rst new file mode 100644 index 000000000..2106d2f47 --- /dev/null +++ b/docs/decisions/0009-tagging-administrators.rst @@ -0,0 +1,37 @@ +9. Taxonomy administrators +========================== + +Context +------- + +Taxonomy Administrators have the right to create, edit, populate, and delete taxonomies available globably for a given instance, or for a specific organization. + +How should these users be identified and their access granted? + +Decision +-------- + +A modified version of "course creator" access will be used to identify Taxonomy Administrators in Studio (ref `get_organizations`_): + +#. Global staff and superusers can create/edit/populate/delete Taxonomies for the instance or for any org key. + +#. Users who can create courses for "any organization" access can create/edit/populate/delete Taxonomies for the instance or for any org key. + +#. Users who can create courses only for specific organizations can create/edit/populate/delete Taxonomies with only these org keys. + + +Permission #1 requires no external access, so can be enforced by the ``openedx_tagging`` app. + +But because permissions #2 + #3 require access to the edx-platform CMS model `CourseCreator`_, this access can only be enforced in Studio, and so will live under `cms.djangoapps.tagging` along with the ``ContentTag`` class. Tagging MVP must work for libraries v1, v2 and courses created in Studio, and so tying these permissions to Studio is reasonable for the MVP. + + +Rejected Alternatives +--------------------- + +Django users & groups +~~~~~~~~~~~~~~~~~~~~~ + +This is a standard way to grant access in Django apps, but it is not used in Open edX. + +.. _get_organizations: https://github.com/openedx/edx-platform/blob/4dc35c73ffa6d6a1dcb6e9ea1baa5bed40721125/cms/djangoapps/contentstore/views/course.py#L1958 +.. CourseCreator: https://github.com/openedx/edx-platform/blob/4dc35c73ffa6d6a1dcb6e9ea1baa5bed40721125/cms/djangoapps/course_creators/models.py#L27 diff --git a/docs/decisions/0010-taxonomy-enable-context.rst b/docs/decisions/0010-taxonomy-enable-context.rst new file mode 100644 index 000000000..841e3b6b3 --- /dev/null +++ b/docs/decisions/0010-taxonomy-enable-context.rst @@ -0,0 +1,56 @@ +10. Taxonomy enabled for context +================================ + +Context +------- + +The MVP specification says that taxonomies need to be able to be enabled/disabled for the following contexts: instance, organization, and course. + +Content Authors must be able to turn taxonomies (instance and org-levels) on/off at the course level. + +Decision +-------- + +Instance +~~~~~~~~ + +A Taxonomy can be enabled/disabled for all contexts using its ``enabled`` flag. + +Organization +~~~~~~~~~~~~ + +A OrgTaxonomy can be enabled/disabled for a single organization by setting its ``org_owner`` foreign key field. OrgTaxonomy will live under `cms.djangoapps.tagging` and so has access to the Organization model and logic in Studio. + +No specific use cases exist yet for marking a single taxonomy for use by multiple organizations. + +Multiple taxonomies with the same name may co-exist in an instance, so separate taxonomies fulfilling the same function can be created and maintained for different organizations. + +Course +~~~~~~ + +All available taxonomies can be enabled/disabled for a given course from the Course Author via the Course's `Advanced Settings`_ using established mechanisms. + +Disabling taxonomies for a course will remove/hide the taxonomy fields from the course edit page and unit edit page(s), and tags will not be shown in Studio for that course. LMS use of tags is outside of this MVP. + +Future versions may add more granularity to these settings, to be determined by user needs. + +Rejected Alternatives +--------------------- + +Course Waffle Flags +~~~~~~~~~~~~~~~~~~~ + +Use Course Waffle Flags to enable/disable all taxonomies for a given course. + +Waffle flags can only be changed by instance superusers, but the MVP specifically requires that content authors have control over this switch. + + +Link courses to taxonomies +~~~~~~~~~~~~~~~~~~~~~~~~~~ + +Link individual courses as enabled/disabled to specific taxonomies. +This was deemed too granular for the MVP, and the data structures and UI can be simplified by using a broader on/off flag. + + +.. _Advanced Settings: https://github.com/openedx/edx-platform/blob/4dc35c73ffa6d6a1dcb6e9ea1baa5bed40721125/cms/djangoapps/models/settings/course_metadata.py#L28 +.. _Course Waffle Flags: https://github.com/openedx/edx-platform/blob/4dc35c73ffa6d6a1dcb6e9ea1baa5bed40721125/openedx/core/djangoapps/waffle_utils/models.py#L14 diff --git a/docs/decisions/0011-tag-changes.rst b/docs/decisions/0011-tag-changes.rst new file mode 100644 index 000000000..6e1ea7d6d --- /dev/null +++ b/docs/decisions/0011-tag-changes.rst @@ -0,0 +1,35 @@ +11. Taxonomy and tag changes +============================ + +Context +------- + +Tagging content may be a labor-intensive, and the data produced is precious, both for human and automated users. Content tags should be structured and namespaced according to the needs of the instance's taxonomy administrators. But taxonomies themselves need to allow for changes: their tags can be overridden with a single import, they can be deleted, reworked, and their rules changed on the fly. + +What happens to the existing content tags if a Taxonomy or Tag is renamed, moved, or deleted? + +Decision +-------- + +Preserve content tag name:value pairs even if the associated taxonomy or tag is removed. +Reflect name:value changes from the linked taxonomy:tag immediately to the user. + +Content tags (through their base ObjectTag class) store a foreign key to their Taxonomy and Tag (if relevant), but they also store a copy of the Taxonomy.name and Tag.value, which can be used if there is no Taxonomy or Tag available. + +We consult the authoritative Taxonomy.name and Tag.value whenever displaying a content tag, so any changes are immediately reflected to the user. + +If a Taxonomy or Tag is deleted, the linked content tags will remain, and cached copies of the name:value pair will be displayed. + +This cached "value" field enables content tags (through their base ObjectTag class) to store free-text tag values, so that the free-text Taxonomy itself need not be modified when new free-text tags are added. + +This extra storage also allows tagged content to be imported independently of a taxonomy. The taxonomy (and appropriate tags) can be constructed later, and content tags validated and re-synced by editing the content tag or by running a maintenance command. + +Rejected Alternatives +--------------------- + +Require foreign keys +~~~~~~~~~~~~~~~~~~~~ + +Require a foreign key from a content tag to Taxonomy for the name, and Tag for the value. + +Only using foreign keys puts the labor-intensive content tag data at risk during taxonomy changes, and requires free-text tags to be made part of a taxonomy. From 2182b552cf7da202ddc87808e51d5dffbfe80789 Mon Sep 17 00:00:00 2001 From: Jillian Vogel Date: Thu, 1 Jun 2023 15:33:23 +0930 Subject: [PATCH 4/9] refactor: moves openedx_learning.contrib.tagging to openedx_tagging.core.tagging as per ADR 0007. --- openedx_tagging/__init__.py | 1 + .../core}/tagging/__init__.py | 0 .../core}/tagging/admin.py | 1 + .../core}/tagging/apps.py | 2 +- .../core}/tagging/migrations/0001_initial.py | 0 .../core}/tagging/migrations/__init__.py | 0 .../core}/tagging/models.py | 11 ++++++----- .../core}/tagging/readme.rst | 0 projects/dev.py | 3 ++- setup.py | 5 +++-- test_settings.py | 1 + tests/openedx_tagging/__init__.py | 0 tests/openedx_tagging/core/__init__.py | 0 tests/openedx_tagging/core/tagging/__init__.py | 0 .../openedx_tagging/core/tagging/test_models.py | 17 +++++++++++++++++ tox.ini | 8 ++++---- 16 files changed, 36 insertions(+), 13 deletions(-) create mode 100644 openedx_tagging/__init__.py rename {openedx_learning/contrib => openedx_tagging/core}/tagging/__init__.py (100%) rename {openedx_learning/contrib => openedx_tagging/core}/tagging/admin.py (99%) rename {openedx_learning/contrib => openedx_tagging/core}/tagging/apps.py (86%) rename {openedx_learning/contrib => openedx_tagging/core}/tagging/migrations/0001_initial.py (100%) rename {openedx_learning/contrib => openedx_tagging/core}/tagging/migrations/__init__.py (100%) rename {openedx_learning/contrib => openedx_tagging/core}/tagging/models.py (94%) rename {openedx_learning/contrib => openedx_tagging/core}/tagging/readme.rst (100%) create mode 100644 tests/openedx_tagging/__init__.py create mode 100644 tests/openedx_tagging/core/__init__.py create mode 100644 tests/openedx_tagging/core/tagging/__init__.py create mode 100644 tests/openedx_tagging/core/tagging/test_models.py diff --git a/openedx_tagging/__init__.py b/openedx_tagging/__init__.py new file mode 100644 index 000000000..6b99f0677 --- /dev/null +++ b/openedx_tagging/__init__.py @@ -0,0 +1 @@ +"""Open edX Tagging app.""" diff --git a/openedx_learning/contrib/tagging/__init__.py b/openedx_tagging/core/tagging/__init__.py similarity index 100% rename from openedx_learning/contrib/tagging/__init__.py rename to openedx_tagging/core/tagging/__init__.py diff --git a/openedx_learning/contrib/tagging/admin.py b/openedx_tagging/core/tagging/admin.py similarity index 99% rename from openedx_learning/contrib/tagging/admin.py rename to openedx_tagging/core/tagging/admin.py index 2e59ec267..e993198a7 100644 --- a/openedx_learning/contrib/tagging/admin.py +++ b/openedx_tagging/core/tagging/admin.py @@ -1,5 +1,6 @@ """ Tagging app admin """ from django.contrib import admin + from .models import TagContent admin.site.register(TagContent) diff --git a/openedx_learning/contrib/tagging/apps.py b/openedx_tagging/core/tagging/apps.py similarity index 86% rename from openedx_learning/contrib/tagging/apps.py rename to openedx_tagging/core/tagging/apps.py index 46f97d066..2cb90974f 100644 --- a/openedx_learning/contrib/tagging/apps.py +++ b/openedx_tagging/core/tagging/apps.py @@ -10,7 +10,7 @@ class TaggingConfig(AppConfig): Configuration for the tagging Django application. """ - name = "openedx_learning.contrib.tagging" + name = "openedx_tagging.core.tagging" verbose_name = "Tagging" default_auto_field = 'django.db.models.BigAutoField' label = "oel_tagging" diff --git a/openedx_learning/contrib/tagging/migrations/0001_initial.py b/openedx_tagging/core/tagging/migrations/0001_initial.py similarity index 100% rename from openedx_learning/contrib/tagging/migrations/0001_initial.py rename to openedx_tagging/core/tagging/migrations/0001_initial.py diff --git a/openedx_learning/contrib/tagging/migrations/__init__.py b/openedx_tagging/core/tagging/migrations/__init__.py similarity index 100% rename from openedx_learning/contrib/tagging/migrations/__init__.py rename to openedx_tagging/core/tagging/migrations/__init__.py diff --git a/openedx_learning/contrib/tagging/models.py b/openedx_tagging/core/tagging/models.py similarity index 94% rename from openedx_learning/contrib/tagging/models.py rename to openedx_tagging/core/tagging/models.py index 9f3a30d6f..8521b65e7 100644 --- a/openedx_learning/contrib/tagging/models.py +++ b/openedx_tagging/core/tagging/models.py @@ -1,6 +1,7 @@ """ Tagging app data models """ from django.db import models + class TagContent(models.Model): """ TODO: This is a basic representation of TagContent, it may change @@ -12,16 +13,16 @@ class TagContent(models.Model): Name-Value pair ----------------- - We use a Field-Value Pair representation, where `name` functions - as the constant that defines the field data set, and `value` functions + We use a Field-Value Pair representation, where `name` functions + as the constant that defines the field data set, and `value` functions as the variable tag lineage that belong to the set. - + Value lineage --------------- - `value` are stored as null character-delimited strings to preserve the tag's lineage. + `value` are stored as null character-delimited strings to preserve the tag's lineage. We use the null character to avoid choosing a character that may exist in a tag's name. - We don't use the Taxonomy's user-facing delimiter so that this delimiter can be changed + We don't use the Taxonomy's user-facing delimiter so that this delimiter can be changed without disrupting stored tags. """ diff --git a/openedx_learning/contrib/tagging/readme.rst b/openedx_tagging/core/tagging/readme.rst similarity index 100% rename from openedx_learning/contrib/tagging/readme.rst rename to openedx_tagging/core/tagging/readme.rst diff --git a/projects/dev.py b/projects/dev.py index 2d50a6119..99903c3f7 100644 --- a/projects/dev.py +++ b/projects/dev.py @@ -35,13 +35,14 @@ "openedx_learning.core.publishing.apps.PublishingConfig", # Learning Contrib Apps "openedx_learning.contrib.media_server.apps.MediaServerConfig", - "openedx_learning.contrib.tagging.apps.TaggingConfig", # Apps that don't belong in this repo in the long term, but are here to make # testing/iteration easier until the APIs stabilize. "olx_importer.apps.OLXImporterConfig", # REST API "rest_framework", "openedx_learning.rest_api.apps.RESTAPIConfig", + # Tagging Core Apps + "openedx_tagging.core.tagging.apps.TaggingConfig", # Debugging "debug_toolbar", diff --git a/setup.py b/setup.py index aba8d0163..4fef021c4 100755 --- a/setup.py +++ b/setup.py @@ -71,9 +71,10 @@ def is_requirement(line): long_description=README + '\n\n' + CHANGELOG, author='David Ormsbee', author_email='dave@tcril.org', - url='https://github.com/ormsbee/openedx-learning', + url='https://github.com/openedx/openedx-learning', packages=[ - 'openedx_learning' + 'openedx_learning', + 'openedx_tagging', ], include_package_data=True, install_requires=load_requirements('requirements/base.in'), diff --git a/test_settings.py b/test_settings.py index 896092ca5..46e52fa59 100644 --- a/test_settings.py +++ b/test_settings.py @@ -39,6 +39,7 @@ def root(*args): "openedx_learning.core.components.apps.ComponentsConfig", "openedx_learning.core.contents.apps.ContentsConfig", "openedx_learning.core.publishing.apps.PublishingConfig", + "openedx_tagging.core.tagging.apps.TaggingConfig", ] LOCALE_PATHS = [ diff --git a/tests/openedx_tagging/__init__.py b/tests/openedx_tagging/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/tests/openedx_tagging/core/__init__.py b/tests/openedx_tagging/core/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/tests/openedx_tagging/core/tagging/__init__.py b/tests/openedx_tagging/core/tagging/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/tests/openedx_tagging/core/tagging/test_models.py b/tests/openedx_tagging/core/tagging/test_models.py new file mode 100644 index 000000000..d1a4d76a4 --- /dev/null +++ b/tests/openedx_tagging/core/tagging/test_models.py @@ -0,0 +1,17 @@ +from django.test.testcases import TestCase + +from openedx_tagging.core.tagging.models import TagContent + + +class TestModelTagContent(TestCase): + """ + Test that TagContent objects can be created and edited. + """ + + def test_tag_content(self): + content_tag = TagContent.objects.create( + content_id="lb:Axim:video:abc", + name="Subject areas", + value="Chemistry", + ) + assert content_tag.id diff --git a/tox.ini b/tox.ini index 44168b281..a1e1936fe 100644 --- a/tox.ini +++ b/tox.ini @@ -70,11 +70,11 @@ deps = -r{toxinidir}/requirements/quality.txt commands = touch tests/__init__.py - pylint openedx_learning tests test_utils manage.py setup.py + pylint openedx_learning openedx_tagging tests test_utils manage.py setup.py rm tests/__init__.py - pycodestyle openedx_learning tests manage.py setup.py - pydocstyle openedx_learning tests manage.py setup.py - isort --check-only --diff --recursive tests test_utils openedx_learning manage.py setup.py test_settings.py + pycodestyle openedx_learning openedx_tagging tests manage.py setup.py + pydocstyle openedx_learning openedx_tagging tests manage.py setup.py + isort --check-only --diff --recursive tests test_utils openedx_learning openedx_tagging manage.py setup.py test_settings.py make selfcheck [testenv:pii_check] From 9f38046d852765755b970f3813837a094fccb732 Mon Sep 17 00:00:00 2001 From: Jillian Vogel Date: Wed, 7 Jun 2023 10:16:16 +0930 Subject: [PATCH 5/9] docs: clarifies when taxonomies are enabled in different contexts --- .../0010-taxonomy-enable-context.rst | 51 +++++++++++++++---- 1 file changed, 40 insertions(+), 11 deletions(-) diff --git a/docs/decisions/0010-taxonomy-enable-context.rst b/docs/decisions/0010-taxonomy-enable-context.rst index 841e3b6b3..6e8f4c8bd 100644 --- a/docs/decisions/0010-taxonomy-enable-context.rst +++ b/docs/decisions/0010-taxonomy-enable-context.rst @@ -11,29 +11,58 @@ Content Authors must be able to turn taxonomies (instance and org-levels) on/off Decision -------- +When is a taxonomy field shown to course authors in a given course? + ++-------------+-----------------------------+--------------------------+-------------------------------+ +| tax.enabled | tax.enabled_for(course.org) | course enables all tax's | Is taxonomy shown for course? | ++=============+=============================+==========================+===============================+ +| True | True | True | True | ++-------------+-----------------------------+--------------------------+-------------------------------+ +| False | True | True | False | ++-------------+-----------------------------+--------------------------+-------------------------------+ +| True | False | True | False | ++-------------+-----------------------------+--------------------------+-------------------------------+ +| True | True | False | False | ++-------------+-----------------------------+--------------------------+-------------------------------+ +| False | True | False | False | ++-------------+-----------------------------+--------------------------+-------------------------------+ +| False | False | True | False | ++-------------+-----------------------------+--------------------------+-------------------------------+ + +.. _Course +Course +~~~~~~ + +We will add a Course Advanced Settings that allows course authors to enable/disable *all available taxonomies* for a given course. + +In order for a given taxonomy to be "available to a course", it must be enabled in the :ref:`Instance` context and the course's :ref:`Organization` context. + +Disabling taxonomies for a course will remove/hide the taxonomy fields from the course edit page and unit edit page(s), and tags will not be shown in Studio for that course. LMS use of tags is outside of this MVP. + +Future versions may add more granularity to these settings, to be determined by user needs. + +.. _Instance Instance ~~~~~~~~ -A Taxonomy can be enabled/disabled for all contexts using its ``enabled`` flag. +Taxonomy contains a boolean ``enabled`` field. + +A Taxonomy can be disabled for all contexts by setting ``enabled = False``. +If ``enabled = True``, then the :ref:`Organization` and :ref:`Course` contexts determine whether a taxonomy will be shown to course authors. +.. _Organization Organization ~~~~~~~~~~~~ -A OrgTaxonomy can be enabled/disabled for a single organization by setting its ``org_owner`` foreign key field. OrgTaxonomy will live under `cms.djangoapps.tagging` and so has access to the Organization model and logic in Studio. +OrgTaxonomy contains a ``org_owner`` field, which is a foreign key to the Organization model. OrgTaxonomy lives under `cms.djangoapps.tagging` and so has access to the Organization model and logic in Studio. + +An OrgTaxonomy is enabled for all organizations if ``org_owner == None``. +If ``org_owner`` is set, then the OrgTaxonomy is only enabled for that org, i.e. only courses in that org will see the taxonomy field. No specific use cases exist yet for marking a single taxonomy for use by multiple organizations. Multiple taxonomies with the same name may co-exist in an instance, so separate taxonomies fulfilling the same function can be created and maintained for different organizations. -Course -~~~~~~ - -All available taxonomies can be enabled/disabled for a given course from the Course Author via the Course's `Advanced Settings`_ using established mechanisms. - -Disabling taxonomies for a course will remove/hide the taxonomy fields from the course edit page and unit edit page(s), and tags will not be shown in Studio for that course. LMS use of tags is outside of this MVP. - -Future versions may add more granularity to these settings, to be determined by user needs. - Rejected Alternatives --------------------- From f91eca0e1cb6d7fe782ad223d6749090976d7260 Mon Sep 17 00:00:00 2001 From: Jillian Vogel Date: Wed, 7 Jun 2023 12:52:28 +0930 Subject: [PATCH 6/9] docs: ref OEP-9 for permissions --- docs/decisions/0009-tagging-administrators.rst | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/docs/decisions/0009-tagging-administrators.rst b/docs/decisions/0009-tagging-administrators.rst index 2106d2f47..652dcd243 100644 --- a/docs/decisions/0009-tagging-administrators.rst +++ b/docs/decisions/0009-tagging-administrators.rst @@ -11,7 +11,7 @@ How should these users be identified and their access granted? Decision -------- -A modified version of "course creator" access will be used to identify Taxonomy Administrators in Studio (ref `get_organizations`_): +In the Studio context, a modified version of "course creator" access will be used to identify Taxonomy Administrators (ref `get_organizations`_): #. Global staff and superusers can create/edit/populate/delete Taxonomies for the instance or for any org key. @@ -24,6 +24,7 @@ Permission #1 requires no external access, so can be enforced by the ``openedx_t But because permissions #2 + #3 require access to the edx-platform CMS model `CourseCreator`_, this access can only be enforced in Studio, and so will live under `cms.djangoapps.tagging` along with the ``ContentTag`` class. Tagging MVP must work for libraries v1, v2 and courses created in Studio, and so tying these permissions to Studio is reasonable for the MVP. +Per `OEP-9`_, ``openedx_tagging`` will allow applications to use the standard Django API to query permissions, for example: ``user.has_perm('openedx_tagging.edit_taxonomy', taxonomy)``, and the appropriate permissions will be applied in that application's context. Rejected Alternatives --------------------- @@ -34,4 +35,5 @@ Django users & groups This is a standard way to grant access in Django apps, but it is not used in Open edX. .. _get_organizations: https://github.com/openedx/edx-platform/blob/4dc35c73ffa6d6a1dcb6e9ea1baa5bed40721125/cms/djangoapps/contentstore/views/course.py#L1958 -.. CourseCreator: https://github.com/openedx/edx-platform/blob/4dc35c73ffa6d6a1dcb6e9ea1baa5bed40721125/cms/djangoapps/course_creators/models.py#L27 +.. _CourseCreator: https://github.com/openedx/edx-platform/blob/4dc35c73ffa6d6a1dcb6e9ea1baa5bed40721125/cms/djangoapps/course_creators/models.py#L27 +.. _OEP-9: https://open-edx-proposals.readthedocs.io/en/latest/best-practices/oep-0009-bp-permissions.html From f51515da704ed74c72665f76c97a43a8d06d8d67 Mon Sep 17 00:00:00 2001 From: Jillian Vogel Date: Wed, 7 Jun 2023 13:11:12 +0930 Subject: [PATCH 7/9] docs: adds ObjectTag, and updates model inheritance plan also fixes up some link typos in ADR 10 --- docs/decisions/0007-tagging-app.rst | 28 +++++++++++++++++-- .../0010-taxonomy-enable-context.rst | 4 +-- 2 files changed, 27 insertions(+), 5 deletions(-) diff --git a/docs/decisions/0007-tagging-app.rst b/docs/decisions/0007-tagging-app.rst index fca6349b4..f35009d6d 100644 --- a/docs/decisions/0007-tagging-app.rst +++ b/docs/decisions/0007-tagging-app.rst @@ -14,8 +14,30 @@ The openedx_tagging data structures and code will stand alone with no dependenci Classes which require dependencies on other Open edX projects should be defined within a ``tagging`` module inside those projects. -For example, here we define ``openedx_tagging.core.models.Taxonomy``, whose data and functionality are self-contained to the openedx_tagging app. However in Studio, we need to be able to limit access to some Taxonomy by organization, using the same "course creator" access which limits course creation for an organization to a defined set of users. +Taxonomy +~~~~~~~~ -So in edx-platform, we create the ``cms.djangoapps.tagging`` app, and it contains ``models.OrgTaxonomy``, which has a 1:1 relationship with an ``openedx_tagging.core.models.Taxonomy``, so that ``openedx_tagging.core.models.Tag`` entries can be created which link to that Taxonomy. +The ``openedx_tagging`` module defines ``openedx_tagging.core.models.Taxonomy``, whose data and functionality are self-contained to the ``openedx_tagging`` app. However in Studio, we need to be able to limit access to some Taxonomy by organization, using the same "course creator" access which limits course creation for an organization to a defined set of users. -To support the org restrictions, `OrgTaxonomy` adds the ``org_owner`` field, a foreign key to the Organization model, which is provided by the edx-organization app. +So in edx-platform, we will create the ``openedx.features.tagging`` app, to contain ``models.OrgTaxonomy``. OrgTaxonomy subclasses ``openedx_tagging.core.models.Taxonomy``, employing Django's `multi-table inheritance`_ feature, which allows the base Tag class to keep foreign keys to the Taxonomy, while allowing OrgTaxonomy to store foreign keys into Studio's Organization table. + +ObjectTag +~~~~~~~~~ + +Similarly, the ``openedx_tagging`` module defined ``openedx_tagging.core.models.ObjectTag``, also self-contained to the +``openedx_tagging`` app. + +But to tag content in the LMS/Studio, we create ``openedx.features.tagging.models.ContentTag``, which subclasses ``ObjectTag``, and can then reference functionality available in the platform code. + +Rejected Alternatives +--------------------- + +Embed in edx-platform +~~~~~~~~~~~~~~~~~~~~~ + +Embedding the logic in edx-platform would provide the content tagging logic specifically required for the MVP. + +However, we plan to extend tagging to other object types (e.g. People) and contexts (e.g. Marketing), and so a generic, standalone library is preferable in the log run. + + +.. _multi-table inheritance: https://docs.djangoproject.com/en/3.2/topics/db/models/#multi-table-inheritance diff --git a/docs/decisions/0010-taxonomy-enable-context.rst b/docs/decisions/0010-taxonomy-enable-context.rst index 6e8f4c8bd..11b486c3d 100644 --- a/docs/decisions/0010-taxonomy-enable-context.rst +++ b/docs/decisions/0010-taxonomy-enable-context.rst @@ -33,7 +33,7 @@ When is a taxonomy field shown to course authors in a given course? Course ~~~~~~ -We will add a Course Advanced Settings that allows course authors to enable/disable *all available taxonomies* for a given course. +We will add a Course `Advanced Settings`_ that allows course authors to enable/disable *all available taxonomies* for a given course. In order for a given taxonomy to be "available to a course", it must be enabled in the :ref:`Instance` context and the course's :ref:`Organization` context. @@ -69,7 +69,7 @@ Rejected Alternatives Course Waffle Flags ~~~~~~~~~~~~~~~~~~~ -Use Course Waffle Flags to enable/disable all taxonomies for a given course. +Use `Course Waffle Flags`_ to enable/disable all taxonomies for a given course. Waffle flags can only be changed by instance superusers, but the MVP specifically requires that content authors have control over this switch. From 152b8fce8e0367664ad3e9491a1b72f453d3be79 Mon Sep 17 00:00:00 2001 From: Jillian Vogel Date: Thu, 8 Jun 2023 12:32:01 +0930 Subject: [PATCH 8/9] docs: updated to reflect "multiple orgs per taxonomy" decision on doc --- docs/decisions/0010-taxonomy-enable-context.rst | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/docs/decisions/0010-taxonomy-enable-context.rst b/docs/decisions/0010-taxonomy-enable-context.rst index 11b486c3d..8e9a854ae 100644 --- a/docs/decisions/0010-taxonomy-enable-context.rst +++ b/docs/decisions/0010-taxonomy-enable-context.rst @@ -6,6 +6,8 @@ Context The MVP specification says that taxonomies need to be able to be enabled/disabled for the following contexts: instance, organization, and course. +Taxonomy Administrators must be able to enable a taxonomy globally for all organizations in an instance, or to set a list of organizations who can use the taxonomy. + Content Authors must be able to turn taxonomies (instance and org-levels) on/off at the course level. Decision @@ -54,18 +56,21 @@ If ``enabled = True``, then the :ref:`Organization` and :ref:`Course` contexts d Organization ~~~~~~~~~~~~ -OrgTaxonomy contains a ``org_owner`` field, which is a foreign key to the Organization model. OrgTaxonomy lives under `cms.djangoapps.tagging` and so has access to the Organization model and logic in Studio. - -An OrgTaxonomy is enabled for all organizations if ``org_owner == None``. -If ``org_owner`` is set, then the OrgTaxonomy is only enabled for that org, i.e. only courses in that org will see the taxonomy field. +OrgTaxonomy has a many-to-many relationship with the Organization model, accessed via the ``org_owners`` field. OrgTaxonomy lives under `cms.djangoapps.tagging` and so has access to the Organization model and logic in Studio. -No specific use cases exist yet for marking a single taxonomy for use by multiple organizations. +An OrgTaxonomy is enabled for all organizations if ``org_owners == []``. +If there are any ``org_owners`` set, then the OrgTaxonomy is only enabled for those orgas, i.e. only courses in these orgs will see the taxonomy field in Studio. -Multiple taxonomies with the same name may co-exist in an instance, so separate taxonomies fulfilling the same function can be created and maintained for different organizations. +Allowing multiple orgs to access a taxonomy reduces redundancy in data and maintenance. Rejected Alternatives --------------------- +Single org per taxonomy +~~~~~~~~~~~~~~~~~~~~~~~ + +Having a single org on a taxonomy is simpler from an implementation perspective, but the UI/UX frames demonstrated that it is simpler for the user to maintain a single taxonomy for multiple orgs. + Course Waffle Flags ~~~~~~~~~~~~~~~~~~~ From 9aa6e989fdf5013479245ec19b13860b347badee Mon Sep 17 00:00:00 2001 From: Jillian Vogel Date: Fri, 9 Jun 2023 14:49:52 +0930 Subject: [PATCH 9/9] docs: fixed nit --- docs/decisions/0007-tagging-app.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/decisions/0007-tagging-app.rst b/docs/decisions/0007-tagging-app.rst index f35009d6d..7dbc45b0b 100644 --- a/docs/decisions/0007-tagging-app.rst +++ b/docs/decisions/0007-tagging-app.rst @@ -17,7 +17,7 @@ Classes which require dependencies on other Open edX projects should be defined Taxonomy ~~~~~~~~ -The ``openedx_tagging`` module defines ``openedx_tagging.core.models.Taxonomy``, whose data and functionality are self-contained to the ``openedx_tagging`` app. However in Studio, we need to be able to limit access to some Taxonomy by organization, using the same "course creator" access which limits course creation for an organization to a defined set of users. +The ``openedx_tagging`` module defines ``openedx_tagging.core.models.Taxonomy``, whose data and functionality are self-contained to the ``openedx_tagging`` app. However in Studio, we need to be able to limit access to some Taxonomy by organization, using the same "course creator" access which limits course creation for an organization to a defined set of users. So in edx-platform, we will create the ``openedx.features.tagging`` app, to contain ``models.OrgTaxonomy``. OrgTaxonomy subclasses ``openedx_tagging.core.models.Taxonomy``, employing Django's `multi-table inheritance`_ feature, which allows the base Tag class to keep foreign keys to the Taxonomy, while allowing OrgTaxonomy to store foreign keys into Studio's Organization table.