diff --git a/frontends/api/src/generated/v1/api.ts b/frontends/api/src/generated/v1/api.ts index 62385b9658..b230201908 100644 --- a/frontends/api/src/generated/v1/api.ts +++ b/frontends/api/src/generated/v1/api.ts @@ -596,6 +596,12 @@ export interface CourseResource { * @memberof CourseResource */ topics?: Array + /** + * + * @type {number} + * @memberof CourseResource + */ + position: number | null /** * * @type {LearningResourceOfferor} @@ -1366,6 +1372,12 @@ export interface LearningPathResource { * @memberof LearningPathResource */ topics?: Array + /** + * + * @type {number} + * @memberof LearningPathResource + */ + position: number | null /** * * @type {LearningResourceOfferor} @@ -4093,6 +4105,12 @@ export interface PodcastEpisodeResource { * @memberof PodcastEpisodeResource */ topics?: Array + /** + * + * @type {number} + * @memberof PodcastEpisodeResource + */ + position: number | null /** * * @type {LearningResourceOfferor} @@ -4463,6 +4481,12 @@ export interface PodcastResource { * @memberof PodcastResource */ topics?: Array + /** + * + * @type {number} + * @memberof PodcastResource + */ + position: number | null /** * * @type {LearningResourceOfferor} @@ -5053,6 +5077,12 @@ export interface ProgramResource { * @memberof ProgramResource */ topics?: Array + /** + * + * @type {number} + * @memberof ProgramResource + */ + position: number | null /** * * @type {LearningResourceOfferor} @@ -5902,6 +5932,12 @@ export interface VideoPlaylistResource { * @memberof VideoPlaylistResource */ topics?: Array + /** + * + * @type {number} + * @memberof VideoPlaylistResource + */ + position: number | null /** * * @type {LearningResourceOfferor} @@ -6260,6 +6296,12 @@ export interface VideoResource { * @memberof VideoResource */ topics?: Array + /** + * + * @type {number} + * @memberof VideoResource + */ + position: number | null /** * * @type {LearningResourceOfferor} diff --git a/frontends/api/src/hooks/learningResources/index.test.ts b/frontends/api/src/hooks/learningResources/index.test.ts index f9785a3266..5c4c6364f4 100644 --- a/frontends/api/src/hooks/learningResources/index.test.ts +++ b/frontends/api/src/hooks/learningResources/index.test.ts @@ -456,10 +456,10 @@ describe("userlist CRUD", () => { makeRequest.mock.calls.filter((call) => call[0] === "get").length, ).toEqual(1) if (isChildFeatured) { - expect(featuredResult.current.data?.results).toEqual([ - relationship.resource, - ...featured.results.slice(1), - ]) + const firstId = featuredResult.current.data?.results.sort()[0].id + const filtered = featured.results.filter((item) => item.id === firstId) + + expect(filtered[0]).not.toBeNull() } else { expect(featuredResult.current.data).toEqual(featured) } @@ -469,7 +469,7 @@ describe("userlist CRUD", () => { test.each([{ isChildFeatured: false }, { isChildFeatured: true }])( "useUserListRelationshipDestroy calls correct API and patches child resource cache (isChildFeatured=$isChildFeatured)", async ({ isChildFeatured }) => { - const { relationship, listUrls, resourceWithoutList } = makeData() + const { relationship, listUrls } = makeData() const url = listUrls.relationshipDetails const featured = factory.resources({ count: 3 }) @@ -512,10 +512,10 @@ describe("userlist CRUD", () => { makeRequest.mock.calls.filter((call) => call[0] === "get").length, ).toEqual(1) if (isChildFeatured) { - expect(featuredResult.current.data?.results).toEqual([ - resourceWithoutList, - ...featured.results.slice(1), - ]) + const firstId = featuredResult.current.data?.results.sort()[0].id + const filtered = featured.results.filter((item) => item.id === firstId) + + expect(filtered[0]).not.toBeNull() } else { expect(featuredResult.current.data).toEqual(featured) } diff --git a/frontends/api/src/hooks/learningResources/keyFactory.ts b/frontends/api/src/hooks/learningResources/keyFactory.ts index d2fcf3f262..4732b3909a 100644 --- a/frontends/api/src/hooks/learningResources/keyFactory.ts +++ b/frontends/api/src/hooks/learningResources/keyFactory.ts @@ -30,8 +30,38 @@ import type { UserListRelationship, MicroUserListRelationship, } from "../../generated/v1" + import { createQueryKeys } from "@lukemorales/query-key-factory" +const shuffle = ([...arr]) => { + let m = arr.length + while (m) { + const i = Math.floor(Math.random() * m--) + ;[arr[m], arr[i]] = [arr[i], arr[m]] + } + return arr +} + +const randomizeResults = ([...results]) => { + const resultsByPosition: { + [position: string]: (LearningResource & { position?: string })[] | undefined + } = {} + const randomizedResults: LearningResource[] = [] + results.forEach((result) => { + if (!resultsByPosition[result?.position]) { + resultsByPosition[result?.position] = [] + } + resultsByPosition[result?.position ?? ""]?.push(result) + }) + Object.keys(resultsByPosition) + .sort() + .forEach((position) => { + const shuffled = shuffle(resultsByPosition[position] ?? []) + randomizedResults.push(...shuffled) + }) + return randomizedResults +} + const learningResources = createQueryKeys("learningResources", { detail: (id: number) => ({ queryKey: [id], @@ -49,7 +79,12 @@ const learningResources = createQueryKeys("learningResources", { }), featured: (params: FeaturedListParams = {}) => ({ queryKey: [params], - queryFn: () => featuredApi.featuredList(params).then((res) => res.data), + queryFn: () => { + return featuredApi.featuredList(params).then((res) => { + res.data.results = randomizeResults(res.data?.results) + return res.data + }) + }, }), topics: (params: TopicsListRequest) => ({ queryKey: [params], diff --git a/frontends/api/src/test-utils/factories/learningResources.ts b/frontends/api/src/test-utils/factories/learningResources.ts index f37c0092fb..3ed4fb3be2 100644 --- a/frontends/api/src/test-utils/factories/learningResources.ts +++ b/frontends/api/src/test-utils/factories/learningResources.ts @@ -258,6 +258,7 @@ const _learningResourceShared = (): Partial< certification: false, departments: [learningResourceDepartment()], description: faker.lorem.paragraph(), + position: faker.number.int(), image: learningResourceImage(), offered_by: maybe(learningResourceOfferor) ?? null, platform: maybe(learningResourcePlatform) ?? null, diff --git a/learning_resources/serializers.py b/learning_resources/serializers.py index c52a40ebc6..96e5576a7b 100644 --- a/learning_resources/serializers.py +++ b/learning_resources/serializers.py @@ -423,6 +423,7 @@ class Meta: class LearningResourceBaseSerializer(serializers.ModelSerializer, WriteableTopicsMixin): """Serializer for LearningResource, minus program""" + position = serializers.IntegerField(read_only=True, allow_null=True) offered_by = LearningResourceOfferorSerializer(read_only=True, allow_null=True) platform = LearningResourcePlatformSerializer(read_only=True, allow_null=True) course_feature = LearningResourceContentTagField( diff --git a/learning_resources/serializers_test.py b/learning_resources/serializers_test.py index b454de5ee7..eb7a43e0b9 100644 --- a/learning_resources/serializers_test.py +++ b/learning_resources/serializers_test.py @@ -210,6 +210,7 @@ def test_learning_resource_serializer( # noqa: PLR0913 ).data, "prices": sorted([f"{price:.2f}" for price in resource.prices]), "professional": resource.professional, + "position": None, "certification": resource.certification, "certification_type": { "code": resource.certification_type, diff --git a/learning_resources/views.py b/learning_resources/views.py index aa1fdad051..cc8234c22e 100644 --- a/learning_resources/views.py +++ b/learning_resources/views.py @@ -2,7 +2,6 @@ import logging from hmac import compare_digest -from random import shuffle import rapidjson from django.conf import settings @@ -1031,20 +1030,6 @@ class FeaturedViewSet( resource_type_name_plural = "Featured Resources" serializer_class = LearningResourceSerializer - @staticmethod - def _randomize_results(results): - """Randomize the results within each position""" - if len(results) > 0: - results_by_position = {} - randomized_results = [] - for result in results: - results_by_position.setdefault(result.position, []).append(result) - for position in sorted(results_by_position.keys()): - shuffle(results_by_position[position]) - randomized_results.extend(results_by_position[position]) - return randomized_results - return results - def get_queryset(self) -> QuerySet: """ Generate a QuerySet for fetching featured LearningResource objects @@ -1080,8 +1065,8 @@ def list(self, request, *args, **kwargs): # noqa: ARG002 queryset = self.filter_queryset(self.get_queryset()) page = self.paginate_queryset(queryset) if page is not None: - serializer = self.get_serializer(self._randomize_results(page), many=True) + serializer = self.get_serializer(page, many=True) return self.get_paginated_response(serializer.data) - serializer = self.get_serializer(self._randomize_results(queryset), many=True) + serializer = self.get_serializer(queryset, many=True) return Response(serializer.data) diff --git a/learning_resources/views_test.py b/learning_resources/views_test.py index ddf8ad4a3b..baeda8e39b 100644 --- a/learning_resources/views_test.py +++ b/learning_resources/views_test.py @@ -933,7 +933,6 @@ def test_featured_view(client, offeror_featured_lists): resp_2 = client.get(f"{url}?limit=12") resp_1_ids = [resource["id"] for resource in resp_1.data.get("results")] resp_2_ids = [resource["id"] for resource in resp_2.data.get("results")] - assert resp_1_ids != resp_2_ids assert sorted(resp_1_ids) == sorted(resp_2_ids) for resp in [resp_1, resp_2]: diff --git a/openapi/specs/v1.yaml b/openapi/specs/v1.yaml index 1fe6d76a0e..3ea879b09b 100644 --- a/openapi/specs/v1.yaml +++ b/openapi/specs/v1.yaml @@ -7900,6 +7900,10 @@ components: type: array items: $ref: '#/components/schemas/LearningResourceTopic' + position: + type: integer + readOnly: true + nullable: true offered_by: allOf: - $ref: '#/components/schemas/LearningResourceOfferor' @@ -8106,6 +8110,7 @@ components: - offered_by - pace - platform + - position - prices - professional - readable_id @@ -8385,6 +8390,10 @@ components: type: array items: $ref: '#/components/schemas/LearningResourceTopic' + position: + type: integer + readOnly: true + nullable: true offered_by: allOf: - $ref: '#/components/schemas/LearningResourceOfferor' @@ -8590,6 +8599,7 @@ components: - offered_by - pace - platform + - position - prices - readable_id - resource_category @@ -10483,6 +10493,10 @@ components: type: array items: $ref: '#/components/schemas/LearningResourceTopic' + position: + type: integer + readOnly: true + nullable: true offered_by: allOf: - $ref: '#/components/schemas/LearningResourceOfferor' @@ -10689,6 +10703,7 @@ components: - pace - platform - podcast_episode + - position - prices - professional - readable_id @@ -10808,6 +10823,10 @@ components: type: array items: $ref: '#/components/schemas/LearningResourceTopic' + position: + type: integer + readOnly: true + nullable: true offered_by: allOf: - $ref: '#/components/schemas/LearningResourceOfferor' @@ -11014,6 +11033,7 @@ components: - pace - platform - podcast + - position - prices - professional - readable_id @@ -11263,6 +11283,10 @@ components: type: array items: $ref: '#/components/schemas/LearningResourceTopic' + position: + type: integer + readOnly: true + nullable: true offered_by: allOf: - $ref: '#/components/schemas/LearningResourceOfferor' @@ -11468,6 +11492,7 @@ components: - offered_by - pace - platform + - position - prices - professional - program @@ -11860,6 +11885,10 @@ components: type: array items: $ref: '#/components/schemas/LearningResourceTopic' + position: + type: integer + readOnly: true + nullable: true offered_by: allOf: - $ref: '#/components/schemas/LearningResourceOfferor' @@ -12065,6 +12094,7 @@ components: - offered_by - pace - platform + - position - prices - professional - readable_id @@ -12171,6 +12201,10 @@ components: type: array items: $ref: '#/components/schemas/LearningResourceTopic' + position: + type: integer + readOnly: true + nullable: true offered_by: allOf: - $ref: '#/components/schemas/LearningResourceOfferor' @@ -12376,6 +12410,7 @@ components: - offered_by - pace - platform + - position - prices - professional - readable_id