From d675b549db6cd57ee8f023d4f452ab6706b9d23b Mon Sep 17 00:00:00 2001 From: Chris Chudzicki Date: Wed, 1 Oct 2025 13:34:58 -0400 Subject: [PATCH 1/2] create channels with 12 resources by default --- .../management/commands/populate_featured_lists.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/learning_resources/management/commands/populate_featured_lists.py b/learning_resources/management/commands/populate_featured_lists.py index dae2ddfa4a..f6ca6b861e 100644 --- a/learning_resources/management/commands/populate_featured_lists.py +++ b/learning_resources/management/commands/populate_featured_lists.py @@ -31,7 +31,7 @@ def add_arguments(self, parser): "resource_count", nargs="?", type=int, - default=10, + default=12, help="Set the number of courses per featured list (default is 10)", ) @@ -44,7 +44,7 @@ def handle(self, *args, **options): # noqa: ARG002 self.stdout.write("Creating featured list for each featured offeror channel") start = now_in_utc() - resource_count = options.get("resource_count", 10) + resource_count = options["resource_count"] for offeror in LearningResourceOfferor.objects.all(): self.stdout.write(f"Creating featured list for {offeror.name} channel") From 322a01b4bec86fb3a1d150e959ad28c7d0258d19 Mon Sep 17 00:00:00 2001 From: Chris Chudzicki Date: Wed, 1 Oct 2025 13:37:26 -0400 Subject: [PATCH 2/2] fix featured list sorting --- .../src/hooks/learningResources/queries.ts | 42 ++------ .../src/hooks/learningResources/util.test.ts | 97 +++++++++++++++++++ .../api/src/hooks/learningResources/util.ts | 58 +++++++++++ 3 files changed, 165 insertions(+), 32 deletions(-) create mode 100644 frontends/api/src/hooks/learningResources/util.test.ts create mode 100644 frontends/api/src/hooks/learningResources/util.ts diff --git a/frontends/api/src/hooks/learningResources/queries.ts b/frontends/api/src/hooks/learningResources/queries.ts index 5cde01af0f..e9ad586abf 100644 --- a/frontends/api/src/hooks/learningResources/queries.ts +++ b/frontends/api/src/hooks/learningResources/queries.ts @@ -20,6 +20,7 @@ import type { LearningResourcesSearchResponse, } from "../../generated/v1" import { queryOptions } from "@tanstack/react-query" +import { hasPosition, randomizeGroups } from "./util" /* List memberships were previously determined in the learningResourcesApi * from user_list_parents and learning_path_parents on each resource. @@ -37,35 +38,6 @@ export const clearListMemberships = ( learning_path_parents: [], }) -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 learningResourceKeys = { root: ["learning_resources"], // list @@ -183,11 +155,17 @@ const learningResourceQueries = { queryKey: learningResourceKeys.featured(params), queryFn: () => featuredApi.featuredList(params).then((res) => { + const results = res.data.results + const withPosition = results.filter(hasPosition) + if (withPosition.length !== results.length) { + // Should not happen. The featured API always sets position. + console.warn( + "Some featured results are missing position information.", + ) + } return { ...res.data, - results: randomizeResults( - res.data.results.map(clearListMemberships), - ), + results: randomizeGroups(withPosition), } }), }), diff --git a/frontends/api/src/hooks/learningResources/util.test.ts b/frontends/api/src/hooks/learningResources/util.test.ts new file mode 100644 index 0000000000..99ff6f2c12 --- /dev/null +++ b/frontends/api/src/hooks/learningResources/util.test.ts @@ -0,0 +1,97 @@ +import { faker } from "@faker-js/faker/locale/en" +import { randomizeGroups, hasPosition } from "./util" + +faker.seed(12345) // Seed faker for consistent test results +jest + .spyOn(Math, "random") + .mockImplementation(() => faker.number.float({ min: 0, max: 1 })) + +describe("randomizeGroups", () => { + it("should group by position and randomize within groups with duplicates", () => { + const items = [ + { id: "a1", position: 1 }, + { id: "a2", position: 1 }, + { id: "b1", position: 12 }, + { id: "b2", position: 12 }, + { id: "c1", position: 2 }, + { id: "c2", position: 2 }, + { id: "d1", position: 3 }, + ] + + const result = randomizeGroups(items) + + // Should be grouped by position in numerical order + expect(result[0].position).toBe(1) + expect(result[1].position).toBe(1) + expect(result[2].position).toBe(2) + expect(result[3].position).toBe(2) + expect(result[4].position).toBe(3) + expect(result[5].position).toBe(12) + expect(result[6].position).toBe(12) + + // Should contain all items + expect(result).toHaveLength(7) + expect(result.map((item) => item.id).sort()).toEqual([ + "a1", + "a2", + "b1", + "b2", + "c1", + "c2", + "d1", + ]) + }) + + it("should handle positions greater than 10 correctly (avoid lexicographical sorting)", () => { + const items = [ + { id: "item15", position: 15 }, + { id: "item2", position: 2 }, + { id: "item11", position: 11 }, + { id: "item1", position: 1 }, + { id: "item20", position: 20 }, + ] + + const result = randomizeGroups(items) + + // Should be numerically sorted: 1, 2, 11, 15, 20 + // NOT lexicographically sorted: 1, 11, 15, 2, 20 + const positions = result.map((item) => item.position) + expect(positions).toEqual([1, 2, 11, 15, 20]) + }) + + it("should handle empty array", () => { + const items: Array<{ id: string; position: number }> = [] + const result = randomizeGroups(items) + expect(result).toEqual([]) + }) +}) + +describe("hasPosition", () => { + it("should return true for objects with non-null position", () => { + const obj = { id: "test", position: 5 } + expect(hasPosition(obj)).toBe(true) + }) + + it("should return false for objects with null position", () => { + const obj = { id: "test", position: null } + expect(hasPosition(obj)).toBe(false) + }) + + it("should return true for objects with position 0", () => { + const obj = { id: "test", position: 0 } + expect(hasPosition(obj)).toBe(true) + }) + + it("should act as a type guard", () => { + const obj: { id: string; position: number | null } = { + id: "test", + position: 5, + } + + if (hasPosition(obj)) { + // TypeScript should now know that obj.position is number, not number | null + const position: number = obj.position + expect(position).toBe(5) + } + }) +}) diff --git a/frontends/api/src/hooks/learningResources/util.ts b/frontends/api/src/hooks/learningResources/util.ts new file mode 100644 index 0000000000..5e488a663c --- /dev/null +++ b/frontends/api/src/hooks/learningResources/util.ts @@ -0,0 +1,58 @@ +const hasPosition = ( + r: T, +): r is T & { position: number } => r.position !== null + +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 +} + +/** + * Randomize a group of ordered items, where item positions might be duplicated. + * Ordering is preserved between groups, but randomized within groups. + * + * E.g., given the items + * [ + * { id: 1, position: 1 }, + * { id: 2, position: 1 }, + * { id: 3, position: 2 }, + * { id: 4, position: 3 }, + * { id: 5, position: 3 }, + * { id: 6, position: 3 }, + * ] + * + * The results would be: + * [ + * ...items with position 1 in random order..., + * ...items with position 2 in random order..., + * ...items with position 3 in random order... + * ] + */ +const randomizeGroups = (results: T[]): T[] => { + const resultsByPosition: { + [position: string]: T[] | undefined + } = {} + const randomizedResults: T[] = [] + results.forEach((result) => { + const pos = result?.position + if (!resultsByPosition[pos]) { + resultsByPosition[pos] = [] + } + resultsByPosition[pos]?.push(result) + }) + Object.keys(resultsByPosition) + .sort( + (a, b) => Number(a) - Number(b), // Sort positions numerically + ) + .forEach((position) => { + const shuffled = shuffle(resultsByPosition[position] ?? []) + randomizedResults.push(...shuffled) + }) + return randomizedResults +} + +export { randomizeGroups, hasPosition }