Create ChannelVersion model with token support #5589

taoerman · 2025-12-03T18:56:33Z

Summary

Implements the ChannelVersion model to enable version-specific metadata tracking and token-based access for specific channel versions, supporting community library submissions and draft version imports.

References

Fixes: #5460

Reviewer guidance

Run unit tests

AlexVelezLl

Thanks @taoerman! This is looking great! Just few minor/nitpick comments 😄.

AlexVelezLl · 2025-12-03T20:26:33Z

contentcuration/contentcuration/utils/publish.py

+    if is_draft_version:
+        channel_version = ccmodels.ChannelVersion.objects.create(
+            channel=channel, version=None
+        )
+    else:
+        channel_version, created = ccmodels.ChannelVersion.objects.get_or_create(
+            channel=channel, version=channel.version
+        )


I think we can simplify this to

ccmodels.ChannelVersion.objects.get_or_create( channel=channel, version=None if is_draft_version else channel.version )

Fixed, Thanks!

AlexVelezLl · 2025-12-03T20:27:55Z

contentcuration/contentcuration/utils/publish.py

+
+    if not is_draft_version:
+        channel.version += 1
+        channel.save()


Let's rely on just one channel.save at the end of the function, and not here, so that the channel on_update check is only run once we have already created the ChannelVersion object.

Make sense, fixed, thanks!

AlexVelezLl · 2025-12-03T20:33:26Z

contentcuration/contentcuration/utils/publish.py

+def increment_channel_version(channel, is_draft_version=False):
+
+    if not is_draft_version:
+        channel.version += 1
+        channel.save()
+
+    if is_draft_version:
+        channel_version = ccmodels.ChannelVersion.objects.create(
+            channel=channel, version=None
+        )
+    else:
+        channel_version, created = ccmodels.ChannelVersion.objects.get_or_create(
+            channel=channel, version=channel.version
+        )
+
+    if not is_draft_version:
+        channel.version_info = channel_version
+        channel.save()
+
+    if is_draft_version:
+        channel_version.new_token()
+
+    return channel_version


(I know this goes against what I wrote in the issue description.) Now that I look at the code, I think we can just create a different method for the draft version creation? And literally left this function untouched as

def increment_channel_version(channel): channel.version += 1 channel.save()

Since the model here will take care of creating the ChannelVersion object, we can just skip it here.

And let the publish_channel method decide which function it should call. What do you think?

Make sense, its clearer!

AlexVelezLl · 2025-12-03T21:06:16Z

contentcuration/contentcuration/utils/publish.py

+        if special_perms_descriptions:
+            existing_licenses = (
+                ccmodels.AuditedSpecialPermissionsLicense.objects.filter(
+                    description__in=special_perms_descriptions
+                )
+            )
+            existing_descriptions = set(
+                existing_licenses.values_list("description", flat=True)
+            )
+
+            new_licenses = [
+                ccmodels.AuditedSpecialPermissionsLicense(
+                    description=description, distributable=False
+                )
+                for description in special_perms_descriptions
+                if description not in existing_descriptions
+            ]
+
+            if new_licenses:
+                ccmodels.AuditedSpecialPermissionsLicense.objects.bulk_create(
+                    new_licenses, ignore_conflicts=True
+                )
+
+            special_permissions_ids = list(
+                ccmodels.AuditedSpecialPermissionsLicense.objects.filter(
+                    description__in=special_perms_descriptions
+                ).values_list("id", flat=True)
+            )


Apologies, I didn't catch this earlier, but it seems we don't actually need to compute existing_licenses for this. We only need to create the new_licenses object that includes all descriptions without filtering, then do the bulk_create with ignore_conflicts=True, and then query the AuditedSpecialPermissionsLicense with the description__in filter. So we would only need to make two db hits: the bulk_create and the final description__in filter. (Not sure if I explained it well 😅)

Thanks, Alex, make sense!!

AlexVelezLl · 2025-12-03T21:11:22Z

contentcuration/contentcuration/utils/publish.py

+            channel.version_info.special_permissions_included.set(
+                ccmodels.AuditedSpecialPermissionsLicense.objects.filter(
+                    id__in=special_permissions_ids
+                )
+            )


Since we are not actually using the special_permissions_ids we can just avoid fetching the permission_ids and use the description__in filter directly here, or... we can just have the filter query as special_permissions (without doing the values_list call) and then here just do .set(special_permissions).

AlexVelezLl · 2025-12-04T15:13:31Z

contentcuration/contentcuration/models.py

+def validate_kind_count_item(value):
+
+    if not isinstance(value, dict):
+        raise ValidationError("Each kind_count item must be a dictionary")
+
+    if "count" not in value or "kind" not in value:
+        raise ValidationError("Each kind_count item must have 'count' and 'kind' keys")
+
+    if not isinstance(value["count"], int) or value["count"] < 0:
+        raise ValidationError("'count' must be a non-negative integer")
+
+    if not isinstance(value["kind"], str) or not value["kind"]:
+        raise ValidationError("'kind' must be a non-empty string")


Could we use the jsonschema package to do this validation? Just like https://github.com/AlexVelezLl/studio/blob/31039f6c128e2ca1c44687240127b58538c88bd8/contentcuration/contentcuration/viewsets/recommendation.py#L19

Fixed, Thanks!

AlexVelezLl · 2025-12-04T15:16:33Z

contentcuration/contentcuration/models.py

+    kind_count = ArrayField(
+        JSONField(), validators=[validate_kind_count_item], null=True, blank=True
+    )
+    included_licenses = ArrayField(
+        models.IntegerField(choices=[(lic[0], lic[1]) for lic in licenses.LICENSELIST]),
+        null=True,
+        blank=True,
+    )
+    included_categories = ArrayField(
+        models.CharField(
+            max_length=100, choices=[(subj, subj) for subj in subjects.SUBJECTSLIST]
+        ),


Sounds like a good idea if we add separate get_license_choices and get_categories_choices helper functions instead?

Fixed it, Thanks!

contentcuration/contentcuration/tests/test_channel_version.py

AlexVelezLl · 2025-12-04T15:22:16Z

contentcuration/contentcuration/tests/utils/test_publish.py

-        # In reality, the versioned database for the current version
-        # and the unversioned database would have the same content,
-        # but here we provide different content so that we can test
-        # that the versioned database is not overwritten.


I think we can keep this comment

Got it, thanks!

AlexVelezLl · 2025-12-04T15:25:04Z

contentcuration/contentcuration/tests/viewsets/test_channel.py

+        # version-detail returns empty dict when no version_info exists
+        self.assertEqual(response.json(), {})

    def test_get_published_data__is_admin(self):
        self.client.force_authenticate(user=self.admin_user)

        response = self.client.get(
-            reverse("channel-published-data", kwargs={"pk": self.channel.id}),
+            reverse("channel-version-detail", kwargs={"pk": self.channel.id}),
            format="json",
        )
        self.assertEqual(response.status_code, 200, response.content)
-        self.assertEqual(response.json(), self.channel.published_data)
+        # version-detail returns empty dict when no version_info exists
+        self.assertEqual(response.json(), {})

    def test_get_published_data__is_forbidden_user(self):
        self.client.force_authenticate(user=self.forbidden_user)

        response = self.client.get(
-            reverse("channel-published-data", kwargs={"pk": self.channel.id}),
+            reverse("channel-version-detail", kwargs={"pk": self.channel.id}),


Here we should update the test to make sense with the new version detail object. i.e. populate the versionInfo fields, asserting against the most recent ChannelVersion, etc. Also we should rename this test case, since it is still referencing the tests as "test get publish data"

Fixed it, Thanks!

…rt' of https://github.com/taoerman/studio into issue-5460-Create-ChannelVersion-model-with-token-support megr# the commit.

AlexVelezLl

Looking better! I found 500 errors while publishing the channel. I have left some notes for them!

AlexVelezLl · 2025-12-05T14:50:28Z

...ation/frontend/channelEdit/components/sidePanels/SubmitToCommunityLibrarySidePanel/index.vue

      const latestPublishedData = computed(() => {
-        if (!publishedData.value || !displayedVersion.value) return undefined;
-        return publishedData.value[displayedVersion.value];
+        return publishedData.value;
      });


Here, we don't actually need to create a computed property to wrap another computed property without doing any other computation. For this, I think it'd be best just to rename the publishedData property to something like versionDetail and use this property instead of the latestPublishedData property.

Note that there are some fields that have changed, like the community_library_invalid_licenses -> non_distributable_licenses_included and community_library_special_permissions -> special_permissions_included

Got it, I have deleted the redundant computed property and changed the outdated filed names to the new one.

AlexVelezLl · 2025-12-05T15:05:55Z

contentcuration/contentcuration/utils/publish.py

+    new_version_info, created = ccmodels.ChannelVersion.objects.get_or_create(
+        channel=channel, version=channel.version
+    )
+
+    ccmodels.Channel.objects.filter(pk=channel.pk).update(
+        version=channel.version, version_info=new_version_info
+    )


Here we can just skip this because if we call channel.save(), the save method will take care of creating the channelVersion instance here.

So this method would safely be written just like it was before:

def increment_channel_version(channel): channel.version += 1 channel.save()

I tried this way before, but i got error like FAILED contentcuration/tests/viewsets/test_contentnode.py::SyncTestCase::test_copy_contentnode_finalization_does_not_make_publishable - AssertionError: 1 != 0. I will try to figure out what went wrong.

LOL, I know what wrong with my code. I did not define the special_perms_descriptions first. I used special_perms_descriptions=[] first, which has been deleted by linting check.

AlexVelezLl · 2025-12-05T15:06:31Z

contentcuration/contentcuration/utils/publish.py

+    if created:
+        channel_version.new_token()


AlexVelezLl · 2025-12-05T15:23:01Z

contentcuration/contentcuration/utils/publish.py

+        )
+        channel.version_info.save()
+
+        if special_perms_descriptions:


Note that at this point, if this condition is false, then special_perms_descriptions will not be defined, and will raise this error:

[2025-12-05 15:21:35,310: ERROR/ForkPoolWorker-2] local variable 'special_perms_descriptions' referenced before assignment Traceback (most recent call last): File "/Users/alexvelezll/Documents/work/learningequality/repos/studio/contentcuration/contentcuration/viewsets/channel.py", line 536, in publish_from_changes self.publish( File "/Users/alexvelezll/Documents/work/learningequality/repos/studio/contentcuration/contentcuration/viewsets/channel.py", line 563, in publish channel = publish_channel( File "/Users/alexvelezll/.pyenv/versions/3.10.13/lib/python3.10/contextlib.py", line 79, in inner return func(*args, **kwds) File "/Users/alexvelezll/Documents/work/learningequality/repos/studio/contentcuration/contentcuration/utils/publish.py", line 1138, in publish_channel fill_published_fields(channel, version_notes) File "/Users/alexvelezll/Documents/work/learningequality/repos/studio/contentcuration/contentcuration/utils/publish.py", line 1020, in fill_published_fields if special_perms_descriptions: UnboundLocalError: local variable 'special_perms_descriptions' referenced before assignment

Fixed, thanks!

AlexVelezLl · 2025-12-05T15:34:56Z

contentcuration/contentcuration/models.py

+    id = UUIDField(primary_key=True, default=uuid.uuid4)
+    channel = models.ForeignKey(
+        Channel, on_delete=models.CASCADE, related_name="channel_versions"
+    )
+    version = models.PositiveIntegerField(null=True, blank=True)
+    secret_token = models.ForeignKey(
+        SecretToken, on_delete=models.SET_NULL, null=True, blank=True
+    )
+    version_notes = models.TextField(null=True, blank=True)
+    size = models.PositiveIntegerField(null=True, blank=True)
+    date_published = models.DateTimeField(null=True, blank=True)
+    resource_count = models.PositiveIntegerField(null=True, blank=True)
+    kind_count = ArrayField(
+        JSONField(), validators=[validate_kind_count_item], null=True, blank=True
+    )
+    included_licenses = ArrayField(
+        models.IntegerField(choices=get_license_choices()),
+        null=True,
+        blank=True,
+    )
+    included_categories = ArrayField(
+        models.CharField(max_length=100, choices=get_categories_choices()),
+        null=True,
+        blank=True,
+    )
+    included_languages = ArrayField(
+        models.CharField(max_length=100),
+        validators=[validate_language_code],
+        null=True,
+        blank=True,
+    )


Seems like validations are not being triggered on save. It seems that for this, we should call the method self.full_clean on the def save() method.

To ensure that we are catching these oversights ⬆️, could we create some tests on the contencuration/tests/test_models.py module, please? Here we can test all these validations properly!

Added more tests to cover validation!

AlexVelezLl · 2025-12-05T15:54:23Z

contentcuration/contentcuration/utils/publish.py

+        channel.version_info.size = int(channel.published_size)
+        channel.version_info.date_published = channel.last_published
+        channel.version_info.version_notes = version_notes
+        channel.version_info.included_languages = language_list


While checking how the publish workflow works with the full_clean call, I noticed that the language_list included some "null" values as items. And this is because in this line we are missing the .exclude(files__language=None), it is clearly not a regression of this PR, but could you fix it, please? Thanks!

Fixed, Thanks!

AlexVelezLl · 2025-12-05T16:10:39Z

contentcuration/contentcuration/models.py

+    """
+    Validator for language codes in included_languages array.
+    """
+    valid_language_codes = [lang[0] for lang in languages.LANGUAGELIST]


Here, the correct validation would be to use lang.code instead of lang[0]

AlexVelezLl · 2025-12-05T16:25:19Z

contentcuration/contentcuration/models.py

+def validate_kind_count_item(value):
+    try:
+        jsonschema.validate(instance=value, schema=KIND_COUNT_ITEM_SCHEMA)
+    except jsonschema.ValidationError as e:
+        raise ValidationError(str(e))
+
+
+def validate_language_code(value):
+    """
+    Validator for language codes in included_languages array.
+    """
+    valid_language_codes = [lang[0] for lang in languages.LANGUAGELIST]


Also, note that the value passed to these methods is the entire array of values, not only each item.

AlexVelezLl · 2025-12-05T16:39:01Z

contentcuration/contentcuration/viewsets/channel.py

+        version_data["id"] = str(version_data["id"])
+
+        if version_data["date_published"]:
+            version_data["date_published"] = version_data["date_published"].strftime(
+                settings.DATE_TIME_FORMAT
+            )


Here, we can live without reformatting these two fields, we usually handle dates with the format returned from the .values( call. The same with UUID's

AlexVelezLl · 2025-12-05T16:42:04Z

contentcuration/contentcuration/viewsets/channel.py

+        version_data["special_permissions_included"] = list(
+            channel.version_info.special_permissions_included.values_list(
+                "id", flat=True
+            )
+        )


Now that I think about it, since we will do another call to fetch the actual descriptions anyways, then it may probably be a better Idea just to add a channelVersion filter to the AuditedSpecialPermissionsLicenseViewset, what do you think? Would it be a good idea to do this here, or do you prefer a followup issue for this?

I think your idea is better for separation, performance and more flexible. I tried to implement your idea, could you please check it if i need to fix it? Thanks!

Nice! Just note that we will also need to update the frontend here to use this filter instead of the by_ids

…ith-token-support

AlexVelezLl

Starts looking really great! Just a few things left 👐.

AlexVelezLl · 2025-12-05T21:03:01Z

...ation/frontend/channelEdit/components/sidePanels/SubmitToCommunityLibrarySidePanel/index.vue

+        if (versionDetail.value && Object.keys(versionDetail.value).length > 0) {
+          const publishedVersions = Object.keys(versionDetail.value).map(v => parseInt(v, 10));


Noting that now versionDetail is not a mapping object of versions anymore, it is just a single version object. so this algorithm would no longer apply

Make sense!

AlexVelezLl · 2025-12-05T21:04:22Z

...ation/frontend/channelEdit/components/sidePanels/SubmitToCommunityLibrarySidePanel/index.vue

        // We need to filter out null values due to a backend bug
        // causing null values to sometimes be included in the list
-        const languageCodes = latestPublishedData.value?.included_languages.filter(
-          code => code !== null,
-        );
+        const languageCodes = versionDetail.value?.included_languages.filter(code => code !== null);


Since now, by definition, included_languages will only hold valid language codes (by the validator we just wrote in the model) then we can remove this comment, and this filter! 🎉

Fixed, thanks!

AlexVelezLl · 2025-12-05T21:14:20Z

contentcuration/contentcuration/tests/utils/test_publish.py

+        ChannelVersion.objects.filter(channel=self.channel).delete()
+        self.channel.version_info = None


Just curious, why do we need these lines? 😅

Lol i used another way to set up tests. Thanks!

AlexVelezLl · 2025-12-05T21:19:51Z

contentcuration/contentcuration/viewsets/channel.py

+class AuditedSpecialPermissionsLicenseFilter(FilterSet):
+    """Filter for AuditedSpecialPermissionsLicense by channelVersion."""
+
+    channel_version = UUIDFilter(
+        field_name="channel_versions__id",
+        help_text="Filter by ChannelVersion ID",
+    )
+
+    class Meta:
+        model = AuditedSpecialPermissionsLicense
+        fields = ["channel_version"]
+
+
+class AuditedSpecialPermissionsLicenseViewSet(ReadOnlyValuesViewset):
+    """
+    ViewSet for retrieving AuditedSpecialPermissionsLicense objects.
+    Supports filtering by channelVersion to get licenses for a specific channel version.
+    """
+
+    queryset = AuditedSpecialPermissionsLicense.objects.all()
+    permission_classes = [IsAuthenticated]
+    filter_backends = [DjangoFilterBackend]
+    filterset_class = AuditedSpecialPermissionsLicenseFilter
+
+    values = ("id", "description", "distributable")
+
+    field_map = {
+        "id": "id",
+        "description": "description",
+        "distributable": "distributable",
+    }


In theory we created this viewset here right? https://github.com/learningequality/studio/pull/5563/files#diff-0bf7eefe9deb327a221bc831f03f9f1585476b5718cff5c956e67ba71aa17253R30 😅

Could we just update the filter set of that view set?

OMG, I forgot I wrote a AuditedSpecialPermissionsLicenseViewSet before. Thanks!!!!

AlexVelezLl · 2025-12-05T21:27:14Z

contentcuration/contentcuration/viewsets/channel.py

+        version_data["special_permissions_included"] = list(
+            channel.version_info.special_permissions_included.values_list(
+                "id", flat=True
+            )
+        )


Nice! Just note that we will also need to update the frontend here to use this filter instead of the by_ids

taoerman and others added 3 commits December 3, 2025 10:53

Create ChannelVersion model with token support

e4a589d

[pre-commit.ci lite] apply automatic fixes

66f50b7

fix linting

31039f6

AlexVelezLl added this to the Studio: Easy Sharing of Community Channels milestone Dec 3, 2025

AlexVelezLl self-assigned this Dec 3, 2025

AlexVelezLl requested changes Dec 4, 2025

View reviewed changes

taoerman and others added 13 commits December 4, 2025 11:03

fix code

82dad0f

[pre-commit.ci lite] apply automatic fixes

7743900

fix code

50f0355

[pre-commit.ci lite] apply automatic fixes

64311a2

fix code

50398cf

fix code

17c9e4c

[pre-commit.ci lite] apply automatic fixes

4819470

fix test

2573dcd

[pre-commit.ci lite] apply automatic fixes

3339633

fix bug

2a4e5ef

Merge branch 'issue-5460-Create-ChannelVersion-model-with-token-suppo…

54867ca

…rt' of https://github.com/taoerman/studio into issue-5460-Create-ChannelVersion-model-with-token-support megr# the commit.

fix bug

4b73ac1

[pre-commit.ci lite] apply automatic fixes

33e0391

AlexVelezLl reviewed Dec 5, 2025

View reviewed changes

taoerman and others added 4 commits December 5, 2025 12:44

fix code

e355fb1

fix code

9792dab

Merge branch 'unstable' into issue-5460-Create-ChannelVersion-model-w…

ca6856f

…ith-token-support

[pre-commit.ci lite] apply automatic fixes

b213fd0

AlexVelezLl reviewed Dec 5, 2025

View reviewed changes

taoerman and others added 5 commits December 6, 2025 10:36

fix code

609e651

fix linting

9c1fa51

[pre-commit.ci lite] apply automatic fixes

a7c0245

fix linting

458cc76

[pre-commit.ci lite] apply automatic fixes

a3c2798

		if (versionDetail.value && Object.keys(versionDetail.value).length > 0) {
		const publishedVersions = Object.keys(versionDetail.value).map(v => parseInt(v, 10));

		ChannelVersion.objects.filter(channel=self.channel).delete()
		self.channel.version_info = None

Create ChannelVersion model with token support #5589

Are you sure you want to change the base?

Create ChannelVersion model with token support #5589

Uh oh!

Conversation

taoerman commented Dec 3, 2025

Summary

References

Reviewer guidance

Uh oh!

AlexVelezLl left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AlexVelezLl left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment