diff --git a/RELEASE.rst b/RELEASE.rst index 7ce7d3c9f9..5bcde23036 100644 --- a/RELEASE.rst +++ b/RELEASE.rst @@ -1,6 +1,13 @@ Release Notes ============= +Version 0.67.2 +-------------- + +- feat: display MicroMasters® label on MITxOnline program product pages (#3318) +- fix: add sitemap for video and podcast page (#3323) +- Show (and count) only published items in userlists/paths (#3321) + Version 0.67.1 (Released May 11, 2026) -------------- diff --git a/frontends/api/src/generated/v0/api.ts b/frontends/api/src/generated/v0/api.ts index ccc0941c53..434b7e64c5 100644 --- a/frontends/api/src/generated/v0/api.ts +++ b/frontends/api/src/generated/v0/api.ts @@ -2263,7 +2263,7 @@ export interface LearningPath { */ id: number /** - * Return the number of items in the list + * Number of published items in the list. * @type {number} * @memberof LearningPath */ diff --git a/frontends/api/src/generated/v1/api.ts b/frontends/api/src/generated/v1/api.ts index 41265ab6ba..4b885d1546 100644 --- a/frontends/api/src/generated/v1/api.ts +++ b/frontends/api/src/generated/v1/api.ts @@ -3173,7 +3173,7 @@ export interface LearningPath { */ id: number /** - * Return the number of items in the list + * Number of published items in the list. * @type {number} * @memberof LearningPath */ @@ -8941,7 +8941,7 @@ export interface UserList { */ topics?: Array /** - * Return the number of items in the list + * Number of published items in the list. * @type {number} * @memberof UserList */ diff --git a/frontends/main/src/app-pages/ProductPages/ProductPageTemplate.tsx b/frontends/main/src/app-pages/ProductPages/ProductPageTemplate.tsx index 150cd99fc0..ad2c32555e 100644 --- a/frontends/main/src/app-pages/ProductPages/ProductPageTemplate.tsx +++ b/frontends/main/src/app-pages/ProductPages/ProductPageTemplate.tsx @@ -9,6 +9,7 @@ import { Typography, HEADER_HEIGHT, Grid2, + Chip, } from "ol-components" import { DEFAULT_RESOURCE_IMG, useImageWithFallback } from "ol-utilities" import { convertToEmbedUrl, hexToRgba } from "@/common/utils" @@ -271,6 +272,7 @@ export type ResourceInfo = { type ProductPageTemplateProps = { currentBreadcrumbLabel: string + label?: string title: string shortDescription: React.ReactNode imageSrc: string @@ -284,6 +286,7 @@ type ProductPageTemplateProps = { ) const ProductPageTemplate: React.FC = ({ currentBreadcrumbLabel, + label, title, shortDescription, imageSrc, @@ -342,13 +345,24 @@ const ProductPageTemplate: React.FC = ({ title={title} /> - - {title} - + + {label ? ( + + ) : null} + + {title} + + {shortDescription} { delete process.env.NEXT_PUBLIC_POSTHOG_API_KEY }) + describe("Program type label", () => { + test.each(["MicroMasters®", "MicroMasters"])( + "Shows 'MicroMasters®' label when program_type is '%s'", + async (programType) => { + const program = makeProgram({ + ...makeReqs(), + program_type: programType, + }) + const page = makePage({ program_details: program }) + setupApis({ program, page }) + renderWithProviders() + + await screen.findByRole("heading", { name: page.title }) + const programTypeLabel = screen.getByTestId("product-page-label") + expect(programTypeLabel).toBeInTheDocument() + expect( + within(programTypeLabel).getByText("MicroMasters®"), + ).toBeInTheDocument() + }, + ) + + test("Shows no label for unbranded types like 'Series'", async () => { + const program = makeProgram({ ...makeReqs(), program_type: "Series" }) + const page = makePage({ program_details: program }) + setupApis({ program, page }) + renderWithProviders() + + await screen.findByRole("heading", { name: page.title }) + expect(screen.queryByTestId("product-page-label")).not.toBeInTheDocument() + }) + + test("Shows no label when program_type is null", async () => { + const program = makeProgram({ ...makeReqs(), program_type: null }) + const page = makePage({ program_details: program }) + setupApis({ program, page }) + renderWithProviders() + + await screen.findByRole("heading", { name: page.title }) + expect(screen.queryByTestId("product-page-label")).not.toBeInTheDocument() + }) + }) + describe("Stay Updated button", () => { useStayUpdatedEnv() diff --git a/frontends/main/src/app-pages/ProductPages/ProgramPage.tsx b/frontends/main/src/app-pages/ProductPages/ProgramPage.tsx index a5133db254..e9c3cfc689 100644 --- a/frontends/main/src/app-pages/ProductPages/ProgramPage.tsx +++ b/frontends/main/src/app-pages/ProductPages/ProgramPage.tsx @@ -217,6 +217,18 @@ const RequirementsSection: React.FC = ({ ) } +const PROGRAM_TYPE_DISPLAY: Record = { + "MicroMasters®": "MicroMasters®", + MicroMasters: "MicroMasters®", +} + +const formatProgramTypeLabel = ( + programType: string | null | undefined, +): string | undefined => { + if (!programType) return undefined + return PROGRAM_TYPE_DISPLAY[programType] +} + const ProgramPage: React.FC = ({ readableId }) => { const pages = useQuery(pagesQueries.programPages(readableId)) const programs = useQuery( @@ -272,6 +284,7 @@ const ProgramPage: React.FC = ({ readableId }) => { return ( { + it("returns expected sitemap params", async () => { + const pages = faker.number.int({ min: 4, max: 6 }) + const summaries = factories.learningResources.resourceSummaries({ + count: pages * 1_000 - 350, + pageSize: 1, + }) + + setMockResponse.get( + urls.learningResources.summaryList({ + limit: 1, + resource_type: RESOURCE_TYPES, + }), + summaries, + ) + + const result = await generateSitemaps() + expect(result).toHaveLength(pages) + expect(result).toEqual( + new Array(pages).fill(null).map((_, index) => ({ + id: index, + location: `http://test.learn.odl.local:8062/sitemaps/podcast/sitemap/${index}.xml`, + })), + ) + }) + + it("generates expected URLs for podcast resources", async () => { + const page = faker.number.int({ min: 5, max: 10 }) + const podcastList = factories.learningResources.podcasts({ + count: 3, + pageSize: 3, + }) + + setMockResponse.get( + urls.learningResources.list({ + limit: 1_000, + offset: page * 1_000, + resource_type: RESOURCE_TYPES, + }), + podcastList, + ) + + const sitemapPage = await sitemap({ id: String(page) }) + expect(sitemapPage).toEqual( + podcastList.results.map((resource) => ({ + url: `http://test.learn.odl.local:8062/podcast/${resource.id}`, + lastModified: resource.last_modified ?? undefined, + })), + ) + }) + + it("generates expected URLs for podcast episode resources", async () => { + // Use a non-overlapping range from the podcast test (which uses { min: 5, max: 10 }) + // to avoid query cache collisions on the same list URL. + const page = faker.number.int({ min: 11, max: 20 }) + const podcastId1 = String(faker.number.int()) + const podcastId2 = String(faker.number.int()) + const episodeWithMultipleParents = + factories.learningResources.podcastEpisode({ + podcast_episode: { podcasts: [podcastId1, podcastId2] }, + }) + const episodeWithOneParent = factories.learningResources.podcastEpisode({ + podcast_episode: { podcasts: [podcastId1] }, + }) + const episodeWithoutParent = factories.learningResources.podcastEpisode({ + podcast_episode: { podcasts: [] }, + }) + const results = [ + episodeWithMultipleParents, + episodeWithOneParent, + episodeWithoutParent, + ] + + setMockResponse.get( + urls.learningResources.list({ + limit: 1_000, + offset: page * 1_000, + resource_type: RESOURCE_TYPES, + }), + { count: results.length, next: null, previous: null, results }, + ) + + const sitemapPage = await sitemap({ id: String(page) }) + // episodeWithoutParent should be excluded; episodeWithMultipleParents emits one entry per parent + expect(sitemapPage).toEqual([ + { + url: `http://test.learn.odl.local:8062/podcast/${podcastId1}/podcast_episode/${episodeWithMultipleParents.id}`, + lastModified: episodeWithMultipleParents.last_modified ?? undefined, + }, + { + url: `http://test.learn.odl.local:8062/podcast/${podcastId2}/podcast_episode/${episodeWithMultipleParents.id}`, + lastModified: episodeWithMultipleParents.last_modified ?? undefined, + }, + { + url: `http://test.learn.odl.local:8062/podcast/${podcastId1}/podcast_episode/${episodeWithOneParent.id}`, + lastModified: episodeWithOneParent.last_modified ?? undefined, + }, + ]) + }) +}) diff --git a/frontends/main/src/app/sitemaps/podcast/sitemap.ts b/frontends/main/src/app/sitemaps/podcast/sitemap.ts new file mode 100644 index 0000000000..3257123ef7 --- /dev/null +++ b/frontends/main/src/app/sitemaps/podcast/sitemap.ts @@ -0,0 +1,81 @@ +import type { MetadataRoute } from "next" +import { getQueryClient } from "@/app/getQueryClient" +import { learningResourceQueries } from "api/hooks/learningResources" +import { ResourceTypeEnum } from "api" +import invariant from "tiny-invariant" +import type { GenerateSitemapResult } from "../types" +import { dangerouslyDetectProductionBuildPhase } from "../util" + +const BASE_URL = process.env.NEXT_PUBLIC_ORIGIN +invariant(BASE_URL, "NEXT_PUBLIC_ORIGIN must be defined") + +const PAGE_SIZE = 1_000 + +const RESOURCE_TYPES = [ + ResourceTypeEnum.Podcast, + ResourceTypeEnum.PodcastEpisode, +] + +/** + * As of NextJS 15.5.3, sitemaps are ALWAYS generated at build time, even with + * the force-dynamic below (this may be a NextJS bug?). However, the + * force-dynamic does force re-generation when requests are made in production. + */ +export const dynamic = "force-dynamic" + +export async function generateSitemaps(): Promise { + /** + * NextJS runs this at build time (despite force-dynamic above). + * Early exit here to avoid the useless build-time API calls. + */ + if (dangerouslyDetectProductionBuildPhase()) return [] + + const queryClient = getQueryClient() + const { count } = await queryClient.fetchQuery( + learningResourceQueries.summaryList({ + limit: 1, + resource_type: RESOURCE_TYPES, + }), + ) + + const pages = Math.ceil(count / PAGE_SIZE) + + return new Array(pages).fill(null).map((_, index) => ({ + id: index, + location: `${BASE_URL}/sitemaps/podcast/sitemap/${index}.xml`, + })) +} + +export default async function sitemap({ + id, +}: { + id: string +}): Promise { + const queryClient = getQueryClient() + const data = await queryClient.fetchQuery( + learningResourceQueries.list({ + limit: PAGE_SIZE, + offset: +id * PAGE_SIZE, + resource_type: RESOURCE_TYPES, + }), + ) + + return data.results.flatMap((resource) => { + if (resource.resource_type === ResourceTypeEnum.Podcast) { + return [ + { + url: `${BASE_URL}/podcast/${resource.id}`, + lastModified: resource.last_modified ?? undefined, + }, + ] + } + if (resource.resource_type === ResourceTypeEnum.PodcastEpisode) { + const parentPodcastIds = resource.podcast_episode?.podcasts ?? [] + return parentPodcastIds.map((parentPodcastId) => ({ + url: `${BASE_URL}/podcast/${parentPodcastId}/podcast_episode/${resource.id}`, + lastModified: resource.last_modified ?? undefined, + })) + } + return [] + }) +} diff --git a/frontends/main/src/app/sitemaps/sitemap-index.xml/route.ts b/frontends/main/src/app/sitemaps/sitemap-index.xml/route.ts index fb024f363d..7fd20af808 100644 --- a/frontends/main/src/app/sitemaps/sitemap-index.xml/route.ts +++ b/frontends/main/src/app/sitemaps/sitemap-index.xml/route.ts @@ -3,6 +3,8 @@ import invariant from "tiny-invariant" import * as resourceSitemap from "../resources/sitemap" import * as channelsSitemap from "../channels/sitemap" import * as productsSitemap from "../products/sitemap" +import * as videoSitemap from "../video/sitemap" +import * as podcastSitemap from "../podcast/sitemap" invariant(process.env.NEXT_PUBLIC_ORIGIN, "NEXT_PUBLIC_ORIGIN must be defined") const BASE_URL: string = process.env.NEXT_PUBLIC_ORIGIN @@ -22,6 +24,8 @@ async function buildSitemapIndex(): Promise { resourceSitemap.generateSitemaps(), channelsSitemap.generateSitemaps(), productsSitemap.generateSitemaps(), + videoSitemap.generateSitemaps(), + podcastSitemap.generateSitemaps(), ]) const sitemapUrls = [ `${BASE_URL}/sitemaps/static/sitemap.xml`, diff --git a/frontends/main/src/app/sitemaps/video/sitemap.test.ts b/frontends/main/src/app/sitemaps/video/sitemap.test.ts new file mode 100644 index 0000000000..220a54a2ad --- /dev/null +++ b/frontends/main/src/app/sitemaps/video/sitemap.test.ts @@ -0,0 +1,71 @@ +import { faker } from "@faker-js/faker/locale/en" +import { generateSitemaps, default as sitemap } from "./sitemap" +import { setMockResponse, urls, factories } from "api/test-utils" +import { ResourceTypeEnum } from "api" + +const RESOURCE_TYPES = [ResourceTypeEnum.Video, ResourceTypeEnum.VideoPlaylist] + +describe("Video Sitemaps", () => { + it("returns expected sitemap params", async () => { + const pages = faker.number.int({ min: 4, max: 6 }) + const summaries = factories.learningResources.resourceSummaries({ + count: pages * 1_000 - 350, + pageSize: 1, + }) + + setMockResponse.get( + urls.learningResources.summaryList({ + limit: 1, + resource_type: RESOURCE_TYPES, + }), + summaries, + ) + + const result = await generateSitemaps() + expect(result).toHaveLength(pages) + expect(result).toEqual( + new Array(pages).fill(null).map((_, index) => ({ + id: index, + location: `http://test.learn.odl.local:8062/sitemaps/video/sitemap/${index}.xml`, + })), + ) + }) + + it("generates expected URLs for video and video playlist resources", async () => { + const page = faker.number.int({ min: 5, max: 10 }) + const videoList = factories.learningResources.videos({ + count: 3, + pageSize: 3, + }) + const playlistList = factories.learningResources.videoPlaylists({ + count: 2, + pageSize: 2, + }) + const results = [...videoList.results, ...playlistList.results] + + setMockResponse.get( + urls.learningResources.list({ + limit: 1_000, + offset: page * 1_000, + resource_type: RESOURCE_TYPES, + }), + { count: results.length, next: null, previous: null, results }, + ) + + const sitemapPage = await sitemap({ id: String(page) }) + expect(sitemapPage).toEqual( + results.map((resource) => { + if (resource.resource_type === ResourceTypeEnum.VideoPlaylist) { + return { + url: `http://test.learn.odl.local:8062/video-playlist/${resource.id}`, + lastModified: resource.last_modified ?? undefined, + } + } + return { + url: `http://test.learn.odl.local:8062/video/${resource.id}`, + lastModified: resource.last_modified ?? undefined, + } + }), + ) + }) +}) diff --git a/frontends/main/src/app/sitemaps/video/sitemap.ts b/frontends/main/src/app/sitemaps/video/sitemap.ts new file mode 100644 index 0000000000..42a3077ac4 --- /dev/null +++ b/frontends/main/src/app/sitemaps/video/sitemap.ts @@ -0,0 +1,79 @@ +import type { MetadataRoute } from "next" +import { getQueryClient } from "@/app/getQueryClient" +import { learningResourceQueries } from "api/hooks/learningResources" +import { ResourceTypeEnum } from "api" +import invariant from "tiny-invariant" +import type { GenerateSitemapResult } from "../types" +import { dangerouslyDetectProductionBuildPhase } from "../util" + +const BASE_URL = process.env.NEXT_PUBLIC_ORIGIN +invariant(BASE_URL, "NEXT_PUBLIC_ORIGIN must be defined") + +const PAGE_SIZE = 1_000 + +const RESOURCE_TYPES = [ResourceTypeEnum.Video, ResourceTypeEnum.VideoPlaylist] + +/** + * As of NextJS 15.5.3, sitemaps are ALWAYS generated at build time, even with + * the force-dynamic below (this may be a NextJS bug?). However, the + * force-dynamic does force re-generation when requests are made in production. + */ +export const dynamic = "force-dynamic" + +export async function generateSitemaps(): Promise { + /** + * NextJS runs this at build time (despite force-dynamic above). + * Early exit here to avoid the useless build-time API calls. + */ + if (dangerouslyDetectProductionBuildPhase()) return [] + + const queryClient = getQueryClient() + const { count } = await queryClient.fetchQuery( + learningResourceQueries.summaryList({ + limit: 1, + resource_type: RESOURCE_TYPES, + }), + ) + + const pages = Math.ceil(count / PAGE_SIZE) + + return new Array(pages).fill(null).map((_, index) => ({ + id: index, + location: `${BASE_URL}/sitemaps/video/sitemap/${index}.xml`, + })) +} + +export default async function sitemap({ + id, +}: { + id: string +}): Promise { + const queryClient = getQueryClient() + const data = await queryClient.fetchQuery( + learningResourceQueries.list({ + limit: PAGE_SIZE, + offset: +id * PAGE_SIZE, + resource_type: RESOURCE_TYPES, + }), + ) + + return data.results.flatMap((resource) => { + if (resource.resource_type === ResourceTypeEnum.Video) { + return [ + { + url: `${BASE_URL}/video/${resource.id}`, + lastModified: resource.last_modified ?? undefined, + }, + ] + } + if (resource.resource_type === ResourceTypeEnum.VideoPlaylist) { + return [ + { + url: `${BASE_URL}/video-playlist/${resource.id}`, + lastModified: resource.last_modified ?? undefined, + }, + ] + } + return [] + }) +} diff --git a/learning_resources/models.py b/learning_resources/models.py index 58993ffee5..eb4876615e 100644 --- a/learning_resources/models.py +++ b/learning_resources/models.py @@ -982,8 +982,16 @@ class LearningPathQuerySet(LearningResourceDetailQuerySet): """QuerySet for LearningPath""" def for_serialization(self): - """Prefetch for serialization""" - return self + """Annotate item_count for serialization""" + return self.annotate( + item_count=Count( + "learning_resource__children", + filter=Q( + learning_resource__children__relation_type=LearningResourceRelationTypes.LEARNING_PATH_ITEMS.value, + learning_resource__children__child__published=True, + ), + ) + ) class LearningPath(LearningResourceDetailModel): @@ -1008,13 +1016,28 @@ class LearningPath(LearningResourceDetailModel): def __str__(self): return f"Learning Path: {self.learning_resource.title}" + @cached_property + def item_count(self) -> int: + """Number of published resources in the learning path.""" + return self.learning_resource.children.filter( + relation_type=LearningResourceRelationTypes.LEARNING_PATH_ITEMS.value, + child__published=True, + ).count() + class LearningResourceRelationshipQuerySet(TimestampedModelQuerySet): """QuerySet for LearningResourceRelationship""" def for_serialization(self): """Prefetch related objects used by API serializers""" - return self.select_related("child", "child__image").order_by("position", "id") + return ( + self.select_related("child", "child__image") + .exclude( + relation_type=LearningResourceRelationTypes.LEARNING_PATH_ITEMS.value, + child__published=False, + ) + .order_by("position", "id") + ) class LearningResourceRelationship(TimestampedModel): @@ -1272,6 +1295,11 @@ def first_child_relationship_for_serialization(self): return self.children.order_by("position").first() return children[0] if children else None + @cached_property + def item_count(self) -> int: + """Number of published resources in the list.""" + return self.resources.filter(published=True).count() + class UserListQuerySet(TimestampedModelQuerySet): """QuerySet for UserList""" @@ -1286,9 +1314,9 @@ def for_serialization(self): ), Prefetch( "children", - queryset=UserListRelationship.objects.select_related( - "child", "child__image" - ).order_by("position"), + queryset=UserListRelationship.objects.filter(child__published=True) + .select_related("child", "child__image") + .order_by("position"), to_attr="_children", ), ) @@ -1303,7 +1331,10 @@ class UserListRelationshipQuerySet(TimestampedModelQuerySet): def for_serialization(self): """Prefetch related objects used by API serializers""" return self.select_related("parent", "parent__author").prefetch_related( - Prefetch("child", queryset=LearningResource.objects.for_serialization()) + Prefetch( + "child", + queryset=LearningResource.objects.for_serialization(), + ) ) diff --git a/learning_resources/serializers.py b/learning_resources/serializers.py index ddece6b272..58e5668034 100644 --- a/learning_resources/serializers.py +++ b/learning_resources/serializers.py @@ -397,14 +397,10 @@ class Meta: class ResourceListMixin(serializers.Serializer): """Common fields for LearningPath and other future resource lists""" - item_count = serializers.SerializerMethodField() - - def get_item_count(self, instance) -> int: - """Return the number of items in the list""" - return ( - getattr(instance, "item_count", None) - or instance.learning_resource.children.count() - ) + item_count = serializers.IntegerField( + read_only=True, + help_text="Number of published items in the list.", + ) class CourseNumberSerializer(serializers.Serializer): @@ -1626,7 +1622,10 @@ class UserListSerializer(serializers.ModelSerializer, WriteableTopicsMixin): Simplified serializer for UserList model. """ - item_count = serializers.SerializerMethodField() + item_count = serializers.IntegerField( + read_only=True, + help_text="Number of published items in the list.", + ) image = serializers.SerializerMethodField() def get_image(self, instance) -> dict: @@ -1636,10 +1635,6 @@ def get_image(self, instance) -> dict: return LearningResourceImageSerializer(instance=list_item.child.image).data return None - def get_item_count(self, instance) -> int: - """Return the number of items in the list""" - return getattr(instance, "item_count", None) or instance.resources.count() - def create(self, validated_data): """Create a new user list""" User = get_user_model() diff --git a/learning_resources/views.py b/learning_resources/views.py index 42fac7ea29..fb3383dca1 100644 --- a/learning_resources/views.py +++ b/learning_resources/views.py @@ -979,7 +979,7 @@ class UserListViewSet(viewsets.ModelViewSet): def get_queryset(self): """Return a queryset for this user""" return UserList.objects.for_serialization().annotate( - item_count=Count("children") + item_count=Count("children", filter=Q(children__child__published=True)) ) def list(self, request, **kwargs): # noqa: ARG002 @@ -1053,6 +1053,17 @@ class UserListItemViewSet(NestedViewSetMixin, viewsets.ModelViewSet): http_method_names = VALID_HTTP_METHODS parent_lookup_kwargs = {"userlist_id": "parent"} + def get_queryset(self): + """Hide unpublished children from read actions only. + + Create/update/destroy still see all relationships so users can manage + items whose child resource was unpublished after being added. + """ + queryset = super().get_queryset() + if self.action in ("list", "retrieve"): + queryset = queryset.filter(child__published=True) + return queryset + def create(self, request, *args, **kwargs): # noqa: ARG002 data = request.data.copy() data["parent"] = kwargs.get("userlist_id") diff --git a/learning_resources/views_learningpath_test.py b/learning_resources/views_learningpath_test.py index 64f0e06147..0ff92afab3 100644 --- a/learning_resources/views_learningpath_test.py +++ b/learning_resources/views_learningpath_test.py @@ -345,6 +345,52 @@ def test_learning_path_items_endpoint_delete_items(client, user, is_editor, num_ assert item.position == (old_position - 1 if is_editor else old_position) +def test_learning_path_endpoint_item_count_excludes_unpublished(client, user): + """LearningPath item_count should only include published children""" + update_editor_group(user, True) # noqa: FBT003 + learning_path_res = factories.LearningResourceFactory.create( + is_learning_path=True, published=True + ) + learning_path_res.children.all().delete() + factories.LearningPathRelationshipFactory.create( + parent=learning_path_res, position=1 + ) + factories.LearningPathRelationshipFactory.create( + parent=learning_path_res, position=2, child__published=False + ) + + client.force_login(user) + resp = client.get( + reverse("lr:v1:learningpaths_api-detail", args=[learning_path_res.id]) + ) + assert resp.status_code == 200 + assert resp.data["learning_path"]["item_count"] == 1 + + +def test_learning_path_items_endpoint_excludes_unpublished(client): + """Items list endpoint should only return relationships with published children""" + learning_path = factories.LearningPathFactory.create() + learning_path.learning_resource.children.all().delete() + published_rel = factories.LearningPathRelationshipFactory.create( + parent=learning_path.learning_resource, position=1 + ) + factories.LearningPathRelationshipFactory.create( + parent=learning_path.learning_resource, + position=2, + child__published=False, + ) + + resp = client.get( + reverse( + "lr:v1:learningpathitems_api-list", + args=[learning_path.learning_resource.id], + ) + ) + assert resp.status_code == 200 + results = resp.json()["results"] + assert [item["child"] for item in results] == [published_rel.child_id] + + @pytest.mark.parametrize("is_editor", [True, False]) def test_learning_path_endpoint_delete(client, user, is_editor): """Test learningpath endpoint for deleting a LearningPath""" diff --git a/learning_resources/views_userlist_test.py b/learning_resources/views_userlist_test.py index dd1a9959ab..7bdd7c4c0a 100644 --- a/learning_resources/views_userlist_test.py +++ b/learning_resources/views_userlist_test.py @@ -99,6 +99,39 @@ def _create_list_with_item(): assert expanded_resp.status_code == 200 +def test_user_list_endpoint_item_count_excludes_unpublished(client): + """item_count should only include published resources""" + author = UserFactory.create() + user_list = factories.UserListFactory.create(author=author) + factories.UserListRelationshipFactory.create(parent=user_list, position=1) + factories.UserListRelationshipFactory.create( + parent=user_list, position=2, child__published=False + ) + + client.force_login(author) + resp = client.get(reverse("lr:v1:userlists_api-detail", args=[user_list.id])) + assert resp.status_code == 200 + assert resp.data.get("item_count") == 1 + + +def test_user_list_items_endpoint_excludes_unpublished(client): + """Items list endpoint should only return relationships with published children""" + author = UserFactory.create() + user_list = factories.UserListFactory.create(author=author) + published_rel = factories.UserListRelationshipFactory.create( + parent=user_list, position=1 + ) + factories.UserListRelationshipFactory.create( + parent=user_list, position=2, child__published=False + ) + + client.force_login(author) + resp = client.get(reverse("lr:v1:userlistitems_api-list", args=[user_list.id])) + assert resp.status_code == 200 + results = resp.json()["results"] + assert [item["child"] for item in results] == [published_rel.child_id] + + @pytest.mark.parametrize("is_anonymous", [True, False]) def test_user_list_endpoint_create( # pylint: disable=too-many-arguments client, is_anonymous diff --git a/main/settings.py b/main/settings.py index b85b79e485..c22153921f 100644 --- a/main/settings.py +++ b/main/settings.py @@ -35,7 +35,7 @@ from main.settings_pluggy import * # noqa: F403 from openapi.settings_spectacular import open_spectacular_settings -VERSION = "0.67.1" +VERSION = "0.67.2" log = logging.getLogger() diff --git a/openapi/specs/v0.yaml b/openapi/specs/v0.yaml index b18dda0d69..95f1587eb4 100644 --- a/openapi/specs/v0.yaml +++ b/openapi/specs/v0.yaml @@ -3150,8 +3150,8 @@ components: readOnly: true item_count: type: integer - description: Return the number of items in the list readOnly: true + description: Number of published items in the list. required: - id - item_count diff --git a/openapi/specs/v1.yaml b/openapi/specs/v1.yaml index 7640593fa8..b9f6be1a18 100644 --- a/openapi/specs/v1.yaml +++ b/openapi/specs/v1.yaml @@ -11894,8 +11894,8 @@ components: readOnly: true item_count: type: integer - description: Return the number of items in the list readOnly: true + description: Number of published items in the list. required: - id - item_count @@ -16332,8 +16332,8 @@ components: $ref: '#/components/schemas/LearningResourceTopic' item_count: type: integer - description: Return the number of items in the list readOnly: true + description: Number of published items in the list. image: type: object additionalProperties: {}