Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 17 additions & 13 deletions contentcuration/contentcuration/serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,22 @@ def no_field_eval_repr(self):
serializers.ModelSerializer.__repr__ = no_field_eval_repr


def get_thumbnail_encoding(channel):
"""
Historically, we did not set channel.icon_encoding in the Studio database. We
only set it in the exported Kolibri sqlite db. So when Kolibri asks for the channel
information, fall back to the channel thumbnail data if icon_encoding is not set.
"""
if channel.icon_encoding:
return channel.icon_encoding
if channel.thumbnail_encoding:
base64 = channel.thumbnail_encoding.get("base64")
if base64:
return base64

return None


class PublicChannelSerializer(serializers.ModelSerializer):
"""
Called by the public API, primarily used by Kolibri. Contains information more specific to Kolibri's needs.
Expand All @@ -41,19 +57,7 @@ def match_tokens(self, channel):
)

def get_thumbnail_encoding(self, channel):
"""
Historically, we did not set channel.icon_encoding in the Studio database. We
only set it in the exported Kolibri sqlite db. So when Kolibri asks for the channel
information, fall back to the channel thumbnail data if icon_encoding is not set.
"""
if channel.icon_encoding:
return channel.icon_encoding
if channel.thumbnail_encoding:
base64 = channel.thumbnail_encoding.get("base64")
if base64:
return base64

return None
return get_thumbnail_encoding(channel)

def generate_kind_count(self, channel):
return channel.published_kind_count and json.loads(channel.published_kind_count)
Expand Down
98 changes: 98 additions & 0 deletions contentcuration/kolibri_public/tests/test_public_v1_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
from django.core.cache import cache
from django.urls import reverse

from contentcuration.models import ChannelVersion
from contentcuration.tests.base import BaseAPITestCase
from contentcuration.tests.testdata import generated_base64encoding

Expand Down Expand Up @@ -74,3 +75,100 @@ def test_public_channels_endpoint(self):
self.assertEqual(first_channel["name"], self.channel.name)
self.assertEqual(first_channel["id"], self.channel.id)
self.assertEqual(first_channel["icon_encoding"], generated_base64encoding())

def test_public_channel_lookup_with_channel_version_token_uses_channel_version(
self,
):
"""
A channel version token should resolve to the matched ChannelVersion,
not the channel's current published version.
"""
self.channel.main_tree.published = True
self.channel.main_tree.save()

self.channel.version = 7
self.channel.published_data = {
"2": {"version_notes": "v2 notes"},
"4": {"version_notes": "v4 notes"},
"7": {"version_notes": "v7 notes"},
}
self.channel.save()

channel_version, _created = ChannelVersion.objects.get_or_create(
channel=self.channel,
version=4,
defaults={
"kind_count": [],
"included_languages": [],
"resource_count": 0,
"size": 0,
},
)
version_token = channel_version.new_token().token

lookup_url = reverse(
"get_public_channel_lookup",
kwargs={"version": "v1", "identifier": version_token},
)
response = self.client.get(lookup_url)

self.assertEqual(response.status_code, 200)
self.assertEqual(len(response.data), 1)
self.assertEqual(response.data[0]["version"], 4)
self.assertNotEqual(response.data[0]["version"], self.channel.version)
self.assertEqual(
response.data[0]["version_notes"], {2: "v2 notes", 4: "v4 notes"}
)

def test_public_channel_lookup_channel_version_and_channel_tokens_have_same_keys(
self,
):
"""
Lookup responses from channel-version-token and channel-token endpoints
should expose the same top-level keys, even if values differ.
"""
self.channel.main_tree.published = True
self.channel.main_tree.save()

self.channel.version = 9
self.channel.published_data = {
"3": {"version_notes": "v3 notes"},
"9": {"version_notes": "v9 notes"},
}
self.channel.save()

latest_channel_version, _created = ChannelVersion.objects.get_or_create(
channel=self.channel,
version=9,
defaults={
"kind_count": [],
"included_languages": [],
"resource_count": 0,
"size": 0,
},
)
latest_version_token = latest_channel_version.new_token().token
channel_token = self.channel.make_token().token

channel_version_response = self.client.get(
reverse(
"get_public_channel_lookup",
kwargs={"version": "v1", "identifier": latest_version_token},
)
)
channel_response = self.client.get(
reverse(
"get_public_channel_lookup",
kwargs={"version": "v1", "identifier": channel_token},
)
)

self.assertEqual(channel_version_response.status_code, 200)
self.assertEqual(channel_response.status_code, 200)
self.assertEqual(len(channel_version_response.data), 1)
self.assertEqual(len(channel_response.data), 1)

self.assertSetEqual(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

praise: Good contract test — asserting key parity between the two serialization paths catches drift early. The version-specific filtering in the first test (version_notes only up to version 4) is also well-targeted.

set(channel_version_response.data[0].keys()),
set(channel_response.data[0].keys()),
)
61 changes: 60 additions & 1 deletion contentcuration/kolibri_public/views_v1.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import json
from collections import OrderedDict

from django.conf import settings
from django.contrib.sites.models import Site
Expand All @@ -19,6 +20,8 @@

from contentcuration.decorators import cache_no_user_data
from contentcuration.models import Channel
from contentcuration.models import ChannelVersion
from contentcuration.serializers import get_thumbnail_encoding
from contentcuration.serializers import PublicChannelSerializer


Expand All @@ -28,6 +31,43 @@ def _get_channel_list(version, params, identifier=None):
raise LookupError()


def _get_version_notes(channel, channel_version):
data = {
int(k): v["version_notes"]
for k, v in channel.published_data.items()
if int(k) <= channel_version.version
}
return OrderedDict(sorted(data.items()))


def _serialize_channel_version(channel_version_qs):
channel_version = channel_version_qs.first()
if not channel_version or not channel_version.channel:
return []

channel = channel_version.channel
return [
{
"id": channel.id,
"name": channel.name,
"language": channel.language_id,
"public": channel.public,
"description": channel.description,
"icon_encoding": get_thumbnail_encoding(channel),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

praise: Good call extracting get_thumbnail_encoding to a shared module-level function and importing it here — resolves the duplication concern from the prior review cleanly.

"version_notes": _get_version_notes(channel, channel_version),
"version": channel_version.version,
"kind_count": channel_version.kind_count,
"included_languages": channel_version.included_languages,
"total_resource_count": channel_version.resource_count,
"published_size": channel_version.size,
"last_published": channel_version.date_published,
"matching_tokens": [channel_version.secret_token.token]
if channel_version.secret_token
else [],
}
]


def _get_channel_list_v1(params, identifier=None):
keyword = params.get("keyword", "").strip()
language_id = params.get("language", "").strip()
Expand All @@ -40,6 +80,20 @@ def _get_channel_list_v1(params, identifier=None):
)
if not channels.exists():
channels = Channel.objects.filter(pk=identifier)

if not channels.exists():
# If channels doesnt exist with the given token, check if this is a token of
# a channel version.
channel_version = ChannelVersion.objects.select_related(
"secret_token", "channel"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion: secret_token and channel are ForeignKey fields, not M2M. select_related would be more efficient here — it uses a single JOIN query instead of the separate queries that prefetch_related issues for each FK.

channel_version = ChannelVersion.objects.select_related(
    "secret_token", "channel"
).filter(...)

).filter(
secret_token__token=identifier,
channel__deleted=False,
channel__main_tree__published=True,
)
if channel_version.exists():
# return early as we won't need to apply the other filters for channel version tokens
return channel_version
else:
channels = Channel.objects.prefetch_related("secret_tokens").filter(
Q(public=True) | Q(secret_tokens__token__in=token_list)
Expand Down Expand Up @@ -96,7 +150,12 @@ def get_public_channel_lookup(request, version, identifier):
return HttpResponseNotFound(
_("No channel matching {} found").format(escape(identifier))
)
return Response(PublicChannelSerializer(channel_list, many=True).data)

if channel_list.model == ChannelVersion:
channel_list = _serialize_channel_version(channel_list)
return Response(channel_list)
else:
return Response(PublicChannelSerializer(channel_list, many=True).data)


@api_view(["GET"])
Expand Down
Loading