-
Notifications
You must be signed in to change notification settings - Fork 26
feat: Tagging Django App setup #54
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
Closed
Closed
Changes from all commits
Commits
Show all changes
2 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -64,6 +64,7 @@ requirements/private.txt | |
| dev.db* | ||
|
|
||
| .vscode | ||
| venv/ | ||
|
|
||
| # Media files (for uploads) | ||
| media/ | ||
Empty file.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| """ Tagging app admin """ | ||
| from django.contrib import admin | ||
| from .models import TagContent | ||
|
|
||
| admin.site.register(TagContent) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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" |
23 changes: 23 additions & 0 deletions
23
openedx_learning/contrib/tagging/migrations/0001_initial.py
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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)), | ||
| ], | ||
| ), | ||
| ] |
Empty file.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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). |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Hmm.. this tagging app is supposed to be a "plugin", which to me means:
INSTALLED_APPSlist in the right contextedx-django-utils is specifically tailored to the
lmsandcmsapps in edx-platform, and doesn't have an equivalent on this app, so..Maybe we need to deal with the "pluggability" aspect when it comes time to extract this app out for reuse outside of openedx-learning?
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.
@pomegranited Good point! I didn't account for
edx-django-utilsfor this. I have taken into account that it had to be "installed alongside learning-core as a dependency in the edx-platform"Another option would be to build this app to be installed alongside edx-django-utils so that we can use them with the current open-edx content and install it as a direct dependency on learning-core when we used the learning-core content. The problem is that I don't know how complicated this approach is in the long run.
I will leave this as an open question
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.
Yeah, I think this is something that we can address later. I don't think it needs to be a plugin-utils styled plugin, but more of a library-style app that handles tagging related concerns and is used by our other apps. But I do think this means that the tagging functionality modeling in this repo should be split up so that there is:
openedx_learningpackage. If we're pretty confident that this is going to be its own top level thing that will eventually be used in other parts of the platform, we should give it its own top level package, e.g.openedx_tagging(or some other unique name that we can eventually register on PyPI).openedx_learning.contrib.taggingapp, which would be where we apply tagging toopenedx_learningmodels likePublishableEntityandPublishableEntityVersion. This would probably stay in this repo over the long term.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.
Didn't see this reply when I wrote my message. I like this approach.
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.
I like this approach too, but it has the limitation that edx-django-utils is specifically tailored to edx-platform -- i.e., you can currently only register plugins for "lms" and "cms" -- so we'd need to enhance it to also register plugins for learning core.
So I'd rather not convert this into a plugin right now, but plan for it by respecting the encapsulation @ormsbee laid out above:
openedx_taggingpackage in this repo, sibling ofopenedx_learningopenedx_tagginghas no foreign keys to anything in theopenedx_learningpackageopenedx_learning.contrib.taggingcontains any models that need to link the two packages.@ChrisChV I'm happy to do this move as part of the data architecture ADR which is coming this week, ref openedx/modular-learning#60
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.
I like the approach 👍
Sure, thanks @pomegranited 👍