diff --git a/channels/migrations/0011_add_topic_detail_related_name.py b/channels/migrations/0011_add_topic_detail_related_name.py new file mode 100644 index 0000000000..b0a9a171fb --- /dev/null +++ b/channels/migrations/0011_add_topic_detail_related_name.py @@ -0,0 +1,24 @@ +# Generated by Django 4.2.13 on 2024-07-10 16:43 + +import django.db.models.deletion +from django.db import migrations, models + + +class Migration(migrations.Migration): + dependencies = [ + ("learning_resources", "0055_alter_relationship_options_ordering"), + ("channels", "0010_alter_channel_name_regex"), + ] + + operations = [ + migrations.AlterField( + model_name="channeltopicdetail", + name="topic", + field=models.ForeignKey( + null=True, + on_delete=django.db.models.deletion.SET_NULL, + related_name="channel_topic_details", + to="learning_resources.learningresourcetopic", + ), + ), + ] diff --git a/channels/models.py b/channels/models.py index 22f7dd2308..5d47f30bf2 100644 --- a/channels/models.py +++ b/channels/models.py @@ -4,6 +4,7 @@ from django.core.validators import RegexValidator from django.db import models from django.db.models import JSONField, deletion +from django.db.models.functions import Concat from imagekit.models import ImageSpecField, ProcessedImageField from imagekit.processors import ResizeToFit @@ -14,7 +15,7 @@ LearningResourceOfferor, LearningResourceTopic, ) -from main.models import TimestampedModel +from main.models import TimestampedModel, TimestampedModelQuerySet from main.utils import frontend_absolute_url from profiles.utils import avatar_uri, banner_uri from widgets.models import WidgetList @@ -23,9 +24,27 @@ AVATAR_MEDIUM_MAX_DIMENSION = 90 +class ChannelQuerySet(TimestampedModelQuerySet): + """Custom queryset for Channels""" + + def annotate_channel_url(self): + """Annotate the channel for serialization""" + return self.annotate( + _channel_url=Concat( + models.Value(frontend_absolute_url("/c/")), + "channel_type", + models.Value("/"), + "name", + models.Value("/"), + ) + ) + + class Channel(TimestampedModel): """Channel for any field/subject""" + objects = ChannelQuerySet.as_manager() + # Channel configuration name = models.CharField( max_length=100, @@ -91,13 +110,17 @@ def __str__(self): """Str representation of channel""" return self.title - class Meta: - unique_together = ("name", "channel_type") - @property def channel_url(self) -> str: """Return the channel url""" - return frontend_absolute_url(f"/c/{self.channel_type}/{self.name}/") + return getattr( + self, + "_channel_url", + frontend_absolute_url(f"/c/{self.channel_type}/{self.name}/"), + ) + + class Meta: + unique_together = ("name", "channel_type") class ChannelTopicDetail(TimestampedModel): @@ -110,7 +133,10 @@ class ChannelTopicDetail(TimestampedModel): related_name="topic_detail", ) topic = models.ForeignKey( - LearningResourceTopic, null=True, on_delete=models.SET_NULL + LearningResourceTopic, + null=True, + on_delete=models.SET_NULL, + related_name="channel_topic_details", ) diff --git a/channels/serializers.py b/channels/serializers.py index 7d36f8d14f..2a55d44238 100644 --- a/channels/serializers.py +++ b/channels/serializers.py @@ -147,12 +147,7 @@ def get_lists(self, instance): """Return the channel's list of LearningPaths""" return [ LearningPathPreviewSerializer(channel_list.channel_list).data - for channel_list in ChannelList.objects.filter(channel=instance) - .prefetch_related( - "channel_list", "channel__lists", "channel__featured_list" - ) - .all() - .order_by("position") + for channel_list in instance.lists.all() ] def get_channel_url(self, instance) -> str: diff --git a/channels/views.py b/channels/views.py index 9b7a1bb3e9..41ee1f8e2b 100644 --- a/channels/views.py +++ b/channels/views.py @@ -3,6 +3,7 @@ import logging from django.contrib.auth.models import User +from django.db.models import Prefetch from django_filters.rest_framework import DjangoFilterBackend from drf_spectacular.utils import extend_schema, extend_schema_view from rest_framework import mixins, viewsets @@ -13,7 +14,7 @@ from channels.api import get_group_role_name, remove_user_role from channels.constants import CHANNEL_ROLE_MODERATORS -from channels.models import Channel +from channels.models import Channel, ChannelList from channels.permissions import ChannelModeratorPermissions, HasChannelPermission from channels.serializers import ( ChannelCreateSerializer, @@ -75,8 +76,19 @@ def get_queryset(self): """Return a queryset""" return ( Channel.objects.prefetch_related( - "lists", "sub_channels", "sub_channels__channel" + Prefetch( + "lists", + queryset=ChannelList.objects.prefetch_related( + "channel_list", "channel__lists", "channel__featured_list" + ).order_by("position"), + ), + "sub_channels", + Prefetch( + "sub_channels__channel", + queryset=Channel.objects.annotate_channel_url(), + ), ) + .annotate_channel_url() .select_related( "featured_list", "topic_detail", "department_detail", "unit_detail" ) diff --git a/channels/views_test.py b/channels/views_test.py index 0a0eb4dcaa..024c917279 100644 --- a/channels/views_test.py +++ b/channels/views_test.py @@ -378,7 +378,7 @@ def test_no_excess_queries(user_client, django_assert_num_queries, related_count sub_channels / lists. """ # This isn't too important; we care it does not scale with number of related items - expected_query_count = 11 + expected_query_count = 10 topic_channel = ChannelFactory.create(is_topic=True) ChannelListFactory.create_batch(related_count, channel=topic_channel) diff --git a/conftest.py b/conftest.py index 0ea2a33f4d..ade46839e0 100644 --- a/conftest.py +++ b/conftest.py @@ -20,3 +20,15 @@ def prevent_requests(mocker, request): # noqa: PT004 autospec=True, side_effect=DoNotUseRequestException, ) + + +@pytest.fixture(autouse=True) +def _disable_silky(settings): + """Disable django-silky during tests""" + settings.SILKY_INTERCEPT_PERCENT = 0 + + settings.MIDDLEWARE = tuple( + middleware + for middleware in settings.MIDDLEWARE + if middleware != "silk.middleware.SilkyMiddleware" + ) diff --git a/frontends/api/src/generated/v0/api.ts b/frontends/api/src/generated/v0/api.ts index 305da411e6..e8c175a1dd 100644 --- a/frontends/api/src/generated/v0/api.ts +++ b/frontends/api/src/generated/v0/api.ts @@ -1198,11 +1198,11 @@ export interface LearningResourceTopic { */ parent?: number | null /** - * Get the channel url for the topic if it exists + * * @type {string} * @memberof LearningResourceTopic */ - channel_url: string | null + channel_url: string } /** * Serializer for News FeedItem diff --git a/frontends/api/src/generated/v1/api.ts b/frontends/api/src/generated/v1/api.ts index 73f118a282..121c2e0b33 100644 --- a/frontends/api/src/generated/v1/api.ts +++ b/frontends/api/src/generated/v1/api.ts @@ -2285,11 +2285,11 @@ export interface LearningResourceTopic { */ parent?: number | null /** - * Get the channel url for the topic if it exists + * * @type {string} * @memberof LearningResourceTopic */ - channel_url: string | null + channel_url: string } /** * SearchResponseSerializer with OpenAPI annotations for Learning Resources search diff --git a/learning_resources/models.py b/learning_resources/models.py index 5e1fa5a9ad..eb3d65ea33 100644 --- a/learning_resources/models.py +++ b/learning_resources/models.py @@ -5,7 +5,7 @@ from django.contrib.auth.models import User from django.contrib.postgres.fields import ArrayField from django.db import models -from django.db.models import JSONField, Q +from django.db.models import JSONField, OuterRef, Q from django.db.models.functions import Lower from django.utils import timezone @@ -17,7 +17,7 @@ LearningResourceType, PrivacyLevel, ) -from main.models import TimestampedModel +from main.models import TimestampedModel, TimestampedModelQuerySet def default_learning_format(): @@ -38,11 +38,29 @@ def __str__(self): return f"{self.code}: {self.name}" +class LearningResourceTopicQuerySet(TimestampedModelQuerySet): + """QuerySet for LearningResourceTopic""" + + def annotate_channel_url(self): + """Annotate with the channel url""" + from channels.models import Channel + + return self.annotate( + _channel_url=( + Channel.objects.filter(topic_detail__topic=OuterRef("pk")) + .annotate_channel_url() + .values_list("_channel_url", flat=True)[:1] + ), + ) + + class LearningResourceTopic(TimestampedModel): """ Topics for all learning resources (e.g. "History") """ + objects = LearningResourceTopicQuerySet.as_manager() + name = models.CharField(max_length=128) parent = models.ForeignKey( "LearningResourceTopic", @@ -53,9 +71,18 @@ class LearningResourceTopic(TimestampedModel): def __str__(self): """Return the topic name.""" - return self.name + @property + def channel_url(self): + """Return the topic's channel url""" + if hasattr(self, "_channel_url"): + return self._channel_url + + topic_detail = self.channel_topic_details.first() + + return topic_detail.channel.channel_url if topic_detail is not None else None + class Meta: """Meta options for LearningResourceTopic""" diff --git a/learning_resources/serializers.py b/learning_resources/serializers.py index 5822713078..812100ff59 100644 --- a/learning_resources/serializers.py +++ b/learning_resources/serializers.py @@ -43,12 +43,7 @@ class LearningResourceTopicSerializer(serializers.ModelSerializer): Serializer for LearningResourceTopic model """ - channel_url = serializers.SerializerMethodField(read_only=True, allow_null=True) - - def get_channel_url(self, instance: models.LearningResourceTopic) -> str or None: - """Get the channel url for the topic if it exists""" - channel = Channel.objects.filter(topic_detail__topic=instance).first() - return channel.channel_url if channel else None + channel_url = serializers.CharField() class Meta: """Meta options for the serializer.""" @@ -100,7 +95,7 @@ class LearningResourceOfferorSerializer(serializers.ModelSerializer): channel_url = serializers.SerializerMethodField(read_only=True, allow_null=True) - def get_channel_url(self, instance: models.LearningResourceOfferor) -> str or None: + def get_channel_url(self, instance: models.LearningResourceOfferor) -> str | None: """Get the channel url for the offeror if it exists""" channel = Channel.objects.filter(unit_detail__unit=instance).first() return channel.channel_url if channel else None @@ -160,7 +155,7 @@ class LearningResourceBaseDepartmentSerializer(serializers.ModelSerializer): def get_channel_url( self, instance: models.LearningResourceDepartment - ) -> str or None: + ) -> str | None: """Get the channel url for the department if it exists""" channel = Channel.objects.filter(department_detail__department=instance).first() return channel.channel_url if channel else None diff --git a/learning_resources/views.py b/learning_resources/views.py index 55e2a440e9..01cb00d874 100644 --- a/learning_resources/views.py +++ b/learning_resources/views.py @@ -429,7 +429,9 @@ class TopicViewSet(viewsets.ReadOnlyModelViewSet): Topics covered by learning resources """ - queryset = LearningResourceTopic.objects.all().order_by("name") + queryset = ( + LearningResourceTopic.objects.all().order_by("name").annotate_channel_url() + ) serializer_class = LearningResourceTopicSerializer pagination_class = LargePagination permission_classes = (AnonymousAccessReadonlyPermission,) diff --git a/learning_resources/views_test.py b/learning_resources/views_test.py index fe69c12b8b..5b23f3a06e 100644 --- a/learning_resources/views_test.py +++ b/learning_resources/views_test.py @@ -556,20 +556,24 @@ def test_ocw_webhook_endpoint_bad_key(settings, client): ) -def test_topics_list_endpoint(client): +def test_topics_list_endpoint(client, django_assert_num_queries): """Test topics list endpoint""" topics = sorted( - LearningResourceTopicFactory.create_batch(3), + LearningResourceTopicFactory.create_batch(100), key=lambda topic: topic.name, ) - resp = client.get(reverse("lr:v1:topics_api-list")) - assert resp.data.get("count") == 3 - for i in range(3): - assert ( - resp.data.get("results")[i] - == LearningResourceTopicSerializer(instance=topics[i]).data - ) + with django_assert_num_queries(2): + resp = client.get(reverse("lr:v1:topics_api-list")) + + assert resp.data == { + "count": 100, + "next": None, + "previous": None, + "results": [ + LearningResourceTopicSerializer(instance=topic).data for topic in topics + ], + } def test_topics_detail_endpoint(client): diff --git a/main/settings.py b/main/settings.py index 067b6aa887..279cd0612b 100644 --- a/main/settings.py +++ b/main/settings.py @@ -110,6 +110,7 @@ "news_events", "testimonials", "data_fixtures", + "silk", ) if not get_bool("RUN_DATA_MIGRATIONS", default=False): @@ -150,6 +151,7 @@ "hijack.middleware.HijackUserMiddleware", "oauth2_provider.middleware.OAuth2TokenMiddleware", "django_scim.middleware.SCIMAuthCheckMiddleware", + "silk.middleware.SilkyMiddleware", ) # CORS @@ -713,3 +715,12 @@ def get_all_config_keys(): name="POSTHOG_PROJECT_ID", default=None, ) + +SILKY_INTERCEPT_PERCENT = get_int(name="SILKY_INTERCEPT_PERCENT", default=50) +SILKY_MAX_RECORDED_REQUESTS = get_int(name="SILKY_MAX_RECORDED_REQUESTS", default=10**3) +SILKY_MAX_RECORDED_REQUESTS_CHECK_PERCENT = get_int( + name="SILKY_MAX_RECORDED_REQUESTS_CHECK_PERCENT", default=10 +) +SILKY_AUTHENTICATION = True # User must login +SILKY_AUTHORISATION = True +SILKY_PERMISSIONS = lambda user: user.is_superuser # noqa: E731 diff --git a/main/urls.py b/main/urls.py index aec91e0ff5..536813a3a7 100644 --- a/main/urls.py +++ b/main/urls.py @@ -67,6 +67,7 @@ re_path(r"^hijack/", include("hijack.urls", namespace="hijack")), re_path(r"", include("news_events.urls")), re_path(r"^app", RedirectView.as_view(url=settings.APP_BASE_URL)), + re_path(r"^silk/", include("silk.urls", namespace="silk")), ] + static(settings.MEDIA_URL, document_root=settings.MEDIA_ROOT) if settings.DEBUG: diff --git a/openapi/specs/v0.yaml b/openapi/specs/v0.yaml index f07e27708d..33e08333a3 100644 --- a/openapi/specs/v0.yaml +++ b/openapi/specs/v0.yaml @@ -1485,9 +1485,9 @@ components: maxLength: 12 channel_url: type: string + nullable: true description: Get the channel url for the offeror if it exists readOnly: true - nullable: true name: type: string maxLength: 256 @@ -1607,9 +1607,6 @@ components: nullable: true channel_url: type: string - description: Get the channel url for the topic if it exists - readOnly: true - nullable: true required: - channel_url - id diff --git a/openapi/specs/v1.yaml b/openapi/specs/v1.yaml index 1517ae72fc..08e50522e3 100644 --- a/openapi/specs/v1.yaml +++ b/openapi/specs/v1.yaml @@ -8076,9 +8076,9 @@ components: maxLength: 256 channel_url: type: string + nullable: true description: Get the channel url for the department if it exists readOnly: true - nullable: true required: - channel_url - department_id @@ -8144,9 +8144,9 @@ components: maxLength: 256 channel_url: type: string + nullable: true description: Get the channel url for the department if it exists readOnly: true - nullable: true school: allOf: - $ref: '#/components/schemas/LearningResourceBaseSchool' @@ -8260,9 +8260,9 @@ components: maxLength: 256 channel_url: type: string + nullable: true description: Get the channel url for the offeror if it exists readOnly: true - nullable: true required: - channel_url - code @@ -8276,9 +8276,9 @@ components: maxLength: 12 channel_url: type: string + nullable: true description: Get the channel url for the offeror if it exists readOnly: true - nullable: true name: type: string maxLength: 256 @@ -8670,9 +8670,6 @@ components: nullable: true channel_url: type: string - description: Get the channel url for the topic if it exists - readOnly: true - nullable: true required: - channel_url - id diff --git a/poetry.lock b/poetry.lock index 20b7a964bb..2c2c2eae85 100644 --- a/poetry.lock +++ b/poetry.lock @@ -76,6 +76,20 @@ tests = ["attrs[tests-no-zope]", "zope-interface"] tests-mypy = ["mypy (>=1.6)", "pytest-mypy-plugins"] tests-no-zope = ["attrs[tests-mypy]", "cloudpickle", "hypothesis", "pympler", "pytest (>=4.3.0)", "pytest-xdist[psutil]"] +[[package]] +name = "autopep8" +version = "2.3.1" +description = "A tool that automatically formats Python code to conform to the PEP 8 style guide" +optional = false +python-versions = ">=3.8" +files = [ + {file = "autopep8-2.3.1-py2.py3-none-any.whl", hash = "sha256:a203fe0fcad7939987422140ab17a930f684763bf7335bdb6709991dd7ef6c2d"}, + {file = "autopep8-2.3.1.tar.gz", hash = "sha256:8d6c87eba648fdcfc83e29b788910b8643171c395d9c4bcf115ece035b9c9dda"}, +] + +[package.dependencies] +pycodestyle = ">=2.12.0" + [[package]] name = "backoff" version = "2.2.1" @@ -1123,6 +1137,23 @@ files = [ [package.dependencies] django = "*" +[[package]] +name = "django-silk" +version = "5.1.0" +description = "Silky smooth profiling for the Django Framework" +optional = false +python-versions = ">=3.8" +files = [ + {file = "django-silk-5.1.0.tar.gz", hash = "sha256:34abb5852315f0f3303d45b7ab4a2caa9cf670102b614dbb2ac40a5d2d5cbffb"}, + {file = "django_silk-5.1.0-py3-none-any.whl", hash = "sha256:35a2051672b0be86af4ce734a0df0b6674c8c63f2df730b3756ec6e52923707d"}, +] + +[package.dependencies] +autopep8 = "*" +Django = ">=3.2" +gprof2dot = ">=2017.09.19" +sqlparse = "*" + [[package]] name = "django-storages" version = "1.14.3" @@ -1447,6 +1478,17 @@ protobuf = ">=3.19.5,<3.20.0 || >3.20.0,<3.20.1 || >3.20.1,<4.21.1 || >4.21.1,<4 [package.extras] grpc = ["grpcio (>=1.44.0,<2.0.0.dev0)"] +[[package]] +name = "gprof2dot" +version = "2024.6.6" +description = "Generate a dot graph from the output of several profilers." +optional = false +python-versions = ">=3.8" +files = [ + {file = "gprof2dot-2024.6.6-py2.py3-none-any.whl", hash = "sha256:45b14ad7ce64e299c8f526881007b9eb2c6b75505d5613e96e66ee4d5ab33696"}, + {file = "gprof2dot-2024.6.6.tar.gz", hash = "sha256:fa1420c60025a9eb7734f65225b4da02a10fc6dd741b37fa129bc6b41951e5ab"}, +] + [[package]] name = "greenlet" version = "3.0.3" @@ -2574,6 +2616,17 @@ files = [ [package.dependencies] pyasn1 = ">=0.4.6,<0.7.0" +[[package]] +name = "pycodestyle" +version = "2.12.0" +description = "Python style guide checker" +optional = false +python-versions = ">=3.8" +files = [ + {file = "pycodestyle-2.12.0-py2.py3-none-any.whl", hash = "sha256:949a39f6b86c3e1515ba1787c2022131d165a8ad271b11370a8819aa070269e4"}, + {file = "pycodestyle-2.12.0.tar.gz", hash = "sha256:442f950141b4f43df752dd303511ffded3a04c2b6fb7f65980574f0c31e6e79c"}, +] + [[package]] name = "pycparser" version = "2.22" @@ -4042,4 +4095,4 @@ test = ["big-O", "importlib-resources", "jaraco.functools", "jaraco.itertools", [metadata] lock-version = "2.0" python-versions = "3.12.4" -content-hash = "63a09fdcd2006b42865926ecfdf6ec10351eb7472359887f2aed5dd19f63f78f" +content-hash = "ff8cc40c3b6895f62ada6138a42aba030f14a421860e5813fa6873515339fbab" diff --git a/pyproject.toml b/pyproject.toml index 2ca7c8732f..d2ad8cc0f1 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -79,6 +79,7 @@ django-oauth-toolkit = "^2.3.0" youtube-transcript-api = "^0.6.2" posthog = "^3.5.0" ruff = "0.5.1" +django-silk = "^5.1.0" [tool.poetry.group.dev.dependencies] bpython = "^0.24"