diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index d17a5de1dd..478c2f0ef3 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -56,7 +56,7 @@ jobs: virtualenvs-create: true virtualenvs-in-project: true - - uses: actions/setup-python@82c7e631bb3cdc910f68e0081d67478d79c6982d # v5 + - uses: actions/setup-python@39cd14951b08e74b54015e9e001cdefcf80e669f # v5 with: python-version: "3.12.4" cache: "poetry" @@ -102,7 +102,7 @@ jobs: runs-on: ubuntu-latest steps: - uses: actions/checkout@692973e3d937129bcbf40652eb9f2f61becf3332 # v4 - - uses: actions/setup-node@60edb5dd545a775178f52524783378180af0d1f8 # v4 + - uses: actions/setup-node@1e60f620b9541d16bece96c5465dc8ee9832be0b # v4 with: node-version: "^20" cache: yarn @@ -142,7 +142,7 @@ jobs: NODE_ENV: test - name: Upload frontend build - uses: actions/upload-artifact@65462800fd760344b1a7b4382951275a0abb4808 # v4 + uses: actions/upload-artifact@0b2256b8c012f0828dc542b3febcab082c67f72b # v4 with: name: frontend-build path: frontends/mit-open/build @@ -158,7 +158,7 @@ jobs: - name: Checkout uses: actions/checkout@692973e3d937129bcbf40652eb9f2f61becf3332 # v4 - - uses: actions/setup-node@60edb5dd545a775178f52524783378180af0d1f8 # v4 + - uses: actions/setup-node@1e60f620b9541d16bece96c5465dc8ee9832be0b # v4 with: node-version: "^20" cache: yarn @@ -181,7 +181,7 @@ jobs: runs-on: ubuntu-latest steps: - uses: actions/checkout@692973e3d937129bcbf40652eb9f2f61becf3332 # v4 - - uses: actions/setup-node@60edb5dd545a775178f52524783378180af0d1f8 # v4 + - uses: actions/setup-node@1e60f620b9541d16bece96c5465dc8ee9832be0b # v4 with: node-version: "^20" cache: yarn @@ -220,7 +220,7 @@ jobs: runs-on: ubuntu-latest steps: - uses: actions/checkout@692973e3d937129bcbf40652eb9f2f61becf3332 # v4 - - uses: actions/setup-node@60edb5dd545a775178f52524783378180af0d1f8 # v4 + - uses: actions/setup-node@1e60f620b9541d16bece96c5465dc8ee9832be0b # v4 with: node-version: "^20" cache: yarn @@ -256,7 +256,7 @@ jobs: steps: - uses: actions/checkout@692973e3d937129bcbf40652eb9f2f61becf3332 # v4 - - uses: actions/setup-node@60edb5dd545a775178f52524783378180af0d1f8 # v4 + - uses: actions/setup-node@1e60f620b9541d16bece96c5465dc8ee9832be0b # v4 with: node-version: "^20" cache: yarn @@ -298,7 +298,7 @@ jobs: - name: Upload artifact if: always() - uses: actions/upload-artifact@65462800fd760344b1a7b4382951275a0abb4808 # v4 + uses: actions/upload-artifact@0b2256b8c012f0828dc542b3febcab082c67f72b # v4 with: name: playwright-report path: e2e_testing/playwright-report diff --git a/.github/workflows/production.yml b/.github/workflows/production.yml index fbc1ca38c5..40a20793bf 100644 --- a/.github/workflows/production.yml +++ b/.github/workflows/production.yml @@ -19,7 +19,7 @@ jobs: with: ref: release - - uses: actions/setup-node@60edb5dd545a775178f52524783378180af0d1f8 # v4 + - uses: actions/setup-node@1e60f620b9541d16bece96c5465dc8ee9832be0b # v4 with: node-version: "^20" cache: yarn diff --git a/.github/workflows/publish-pages.yml b/.github/workflows/publish-pages.yml index 4debe4f126..21377f4095 100644 --- a/.github/workflows/publish-pages.yml +++ b/.github/workflows/publish-pages.yml @@ -13,7 +13,7 @@ jobs: - name: Checkout uses: actions/checkout@692973e3d937129bcbf40652eb9f2f61becf3332 # v4 - - uses: actions/setup-node@60edb5dd545a775178f52524783378180af0d1f8 # v4 + - uses: actions/setup-node@1e60f620b9541d16bece96c5465dc8ee9832be0b # v4 with: node-version: "^20" cache: yarn diff --git a/.github/workflows/release-candidate.yml b/.github/workflows/release-candidate.yml index 4d0454255f..923d3e8cbf 100644 --- a/.github/workflows/release-candidate.yml +++ b/.github/workflows/release-candidate.yml @@ -19,7 +19,7 @@ jobs: with: ref: release-candidate - - uses: actions/setup-node@60edb5dd545a775178f52524783378180af0d1f8 # v4 + - uses: actions/setup-node@1e60f620b9541d16bece96c5465dc8ee9832be0b # v4 with: node-version: "^20" cache: yarn diff --git a/RELEASE.rst b/RELEASE.rst index 7c36d22676..94d7219458 100644 --- a/RELEASE.rst +++ b/RELEASE.rst @@ -1,6 +1,16 @@ Release Notes ============= +Version 0.13.22 +--------------- + +- Optimize queries for learning resource APIs +- Fix bug with background image obscuring search controls (#1286) +- Draggable list card styles (#1282) +- Update actions/setup-node digest to 1e60f62 (#1267) +- Update actions/upload-artifact digest to 0b2256b (#1269) +- Update actions/setup-python digest to 39cd149 (#1268) + Version 0.13.21 (Released July 17, 2024) --------------- diff --git a/channels/models.py b/channels/models.py index f1c3b7cec8..6a7b6e8547 100644 --- a/channels/models.py +++ b/channels/models.py @@ -1,5 +1,7 @@ """Models for channels""" +from functools import cached_property + from django.contrib.auth.models import Group from django.core.validators import RegexValidator from django.db import models @@ -30,7 +32,7 @@ class ChannelQuerySet(TimestampedModelQuerySet): def annotate_channel_url(self): """Annotate the channel for serialization""" return self.annotate( - _channel_url=Concat( + channel_url=Concat( models.Value(frontend_absolute_url("/c/")), "channel_type", models.Value("/"), @@ -110,7 +112,7 @@ def __str__(self): """Str representation of channel""" return self.title - @property + @cached_property def channel_url(self) -> str: """Return the channel url""" return getattr( diff --git a/frontends/api/src/generated/v1/api.ts b/frontends/api/src/generated/v1/api.ts index 12d6fc83ca..0565afe383 100644 --- a/frontends/api/src/generated/v1/api.ts +++ b/frontends/api/src/generated/v1/api.ts @@ -4268,10 +4268,10 @@ export type PrivacyLevelEnum = export interface Program { /** * - * @type {Array} + * @type {number} * @memberof Program */ - courses: Array | null + course_count: number } /** * Serializer for Program Certificates diff --git a/frontends/api/src/test-utils/factories/learningResources.ts b/frontends/api/src/test-utils/factories/learningResources.ts index d496c85ce5..7de5efb2e0 100644 --- a/frontends/api/src/test-utils/factories/learningResources.ts +++ b/frontends/api/src/test-utils/factories/learningResources.ts @@ -286,7 +286,7 @@ const program: PartialFactory = (overrides = {}) => { platform: learningResourcePlatform(), certification: faker.datatype.boolean(), program: { - courses: repeat(course, { min: 0, max: 5 }), + course_count: faker.number.int({ min: 0, max: 8 }), }, }, overrides, diff --git a/frontends/mit-open/src/page-components/ItemsListing/ItemsListing.test.tsx b/frontends/mit-open/src/page-components/ItemsListing/ItemsListing.test.tsx index d57fe4c3a5..e791c0d4a7 100644 --- a/frontends/mit-open/src/page-components/ItemsListing/ItemsListing.test.tsx +++ b/frontends/mit-open/src/page-components/ItemsListing/ItemsListing.test.tsx @@ -1,7 +1,11 @@ import React from "react" import { faker } from "@faker-js/faker/locale/en" -import { SortableList, SortableItem } from "ol-components" -import LearningResourceCard from "@/page-components/LearningResourceCard/LearningResourceCard" +import { + SortableList, + SortableItem, + LearningResourceListCard, + LearningResourceListCardCondensed, +} from "ol-components" import { factories, urls, makeRequest } from "api/test-utils" import { screen, @@ -27,10 +31,17 @@ jest.mock("ol-components", () => { ...actual, SortableList: jest.fn(actual.SortableList), SortableItem: jest.fn(actual.SortableItem), + LearningResourceListCard: jest.fn(actual.LearningResourceListCard), + LearningResourceListCardCondensed: jest.fn( + actual.LearningResourceListCardCondensed, + ), } }) -const spyLearningResourceCardOld = jest.mocked(LearningResourceCard) +const spyLearningResourceListCard = jest.mocked(LearningResourceListCard) +const spyLearningResourceListCardCondensed = jest.mocked( + LearningResourceListCardCondensed, +) const spySortableList = jest.mocked(SortableList) const spySortableItem = jest.mocked(SortableItem) @@ -114,21 +125,39 @@ describe("ItemsListing", () => { ) test.each([ - { listType: ListType.LearningPath, sortable: false, cardProps: {} }, + { + listType: ListType.LearningPath, + sortable: false, + condensed: false, + cardProps: {}, + }, { listType: ListType.LearningPath, sortable: true, - cardProps: { sortable: true }, + condensed: false, + cardProps: { draggable: true }, + }, + { + listType: ListType.UserList, + sortable: false, + condensed: false, + cardProps: {}, }, - { listType: ListType.UserList, sortable: false, cardProps: {} }, { listType: ListType.UserList, sortable: true, - cardProps: { sortable: true }, + condensed: false, + cardProps: { draggable: true }, + }, + { + listType: ListType.LearningPath, + sortable: true, + condensed: true, + cardProps: { draggable: true }, }, ])( - "Shows a list of $listType LearningResourceCards with sortable=$sortable", - ({ listType, sortable, cardProps }) => { + "Shows a list of $listType LearningResourceCards with sortable=$sortable and condensed=$condensed", + ({ listType, sortable, condensed, cardProps }) => { const emptyMessage = faker.lorem.sentence() const paginatedRelationships = getPaginatedRelationships( listType, @@ -142,6 +171,7 @@ describe("ItemsListing", () => { emptyMessage={emptyMessage} items={items} sortable={sortable} + condensed={condensed} />, { user: {} }, ) @@ -153,7 +183,14 @@ describe("ItemsListing", () => { if (sortable) { items.forEach(({ resource }) => { - expectProps(spyLearningResourceCardOld, { resource, ...cardProps }) + if (condensed) { + expectProps(spyLearningResourceListCardCondensed, { + resource, + ...cardProps, + }) + } else { + expectProps(spyLearningResourceListCard, { resource, ...cardProps }) + } }) } }, diff --git a/frontends/mit-open/src/page-components/ItemsListing/ItemsListing.tsx b/frontends/mit-open/src/page-components/ItemsListing/ItemsListing.tsx index ffd0c8c05c..0f5eafd120 100644 --- a/frontends/mit-open/src/page-components/ItemsListing/ItemsListing.tsx +++ b/frontends/mit-open/src/page-components/ItemsListing/ItemsListing.tsx @@ -1,6 +1,5 @@ import React, { useCallback, useEffect } from "react" import type { LearningResource } from "api" -import LearningResourceCard from "@/page-components/LearningResourceCard/LearningResourceCard" import { SortableItem, SortableList, @@ -10,6 +9,8 @@ import { LoadingSpinner, styled, PlainList, + LearningResourceListCard, + LearningResourceListCardCondensed, } from "ol-components" import { ResourceListCard } from "@/page-components/ResourceCard/ResourceCard" import { useListItemMove } from "api/hooks/learningResources" @@ -18,6 +19,14 @@ const EmptyMessage = styled.p({ fontStyle: "italic", }) +const Loading = styled.div({ + marginTop: "150px", +}) + +const StyledPlainList = styled(PlainList)({ + marginTop: "8px", +}) + type LearningResourceListItem = { id: number resource: LearningResource @@ -36,30 +45,19 @@ type ItemsListingProps = { condensed?: boolean } -const ItemsListingViewOnly: React.FC<{ - items: NonNullable - condensed?: boolean -}> = ({ items, condensed }) => { - return ( - - {items.map((item) => { - return ( -
  • - -
    • - ) - })} -     - ) -} - const ItemsListingSortable: React.FC<{ listType: NonNullable items: NonNullable isRefetching?: boolean -}> = ({ listType, items, isRefetching }) => { + condensed: boolean +}> = ({ listType, items, isRefetching, condensed }) => { const move = useListItemMove() const [sorted, setSorted] = React.useState([]) + + const ListCardComponent = condensed + ? LearningResourceListCardCondensed + : LearningResourceListCard + /** * `sorted` is a local copy of `items`: * - `onSortEnd`, we'll update `sorted` copy immediately to prevent UI from @@ -68,17 +66,15 @@ const ItemsListingSortable: React.FC<{ * so sync `items` -> `sorted` when `items` changes. */ useEffect(() => setSorted(items), [items]) - const renderDragging: RenderActive = useCallback((active) => { - const item = active.data.current as LearningResourceListItem - return ( - - ) - }, []) + + const renderDragging: RenderActive = useCallback( + (active) => { + const item = active.data.current as LearningResourceListItem + return + }, + [ListCardComponent], + ) + const onSortEnd: OnSortEnd = useCallback( async (e) => { const active = e.active.data @@ -97,9 +93,11 @@ const ItemsListingSortable: React.FC<{ }, [listType, move], ) + const disabled = isRefetching || move.isLoading + return ( - + item.id)} onSortEnd={onSortEnd} @@ -114,23 +112,16 @@ const ItemsListingSortable: React.FC<{ data={item} disabled={disabled} > - {(handleProps) => { - return ( -
    - -
    - ) - }} + {(handleProps) => ( +
    + +
    + )} ) })}     -     + ) } @@ -146,7 +137,9 @@ const ItemsListing: React.FC = ({ return ( <> {isLoading ? ( - + + + ) : items.length === 0 ? ( {emptyMessage} ) : sortable ? ( @@ -154,9 +147,19 @@ const ItemsListing: React.FC = ({ listType={listType} items={items} isRefetching={isRefetching} + condensed={condensed} /> ) : ( - + + {items.map((item) => ( +
  • + +
    • + ))} +     )} ) diff --git a/frontends/mit-open/src/page-components/LearningResourceCard/LearningResourceCard.test.tsx b/frontends/mit-open/src/page-components/LearningResourceCard/LearningResourceCard.test.tsx deleted file mode 100644 index 8714b239ff..0000000000 --- a/frontends/mit-open/src/page-components/LearningResourceCard/LearningResourceCard.test.tsx +++ /dev/null @@ -1,119 +0,0 @@ -import React from "react" -import * as NiceModal from "@ebay/nice-modal-react" -import { renderWithProviders, user, screen } from "../../test-utils" -import type { User } from "../../test-utils" -import LearningResourceCard from "./LearningResourceCard" -import type { LearningResourceCardProps } from "./LearningResourceCard" -import { - AddToLearningPathDialog, - AddToUserListDialog, -} from "../Dialogs/AddToListDialog" -import * as factories from "api/test-utils/factories" -import { RESOURCE_DRAWER_QUERY_PARAM } from "@/common/urls" - -jest.mock("@ebay/nice-modal-react", () => { - const actual = jest.requireActual("@ebay/nice-modal-react") - return { - __esModule: true, - ...actual, - show: jest.fn(), - } -}) - -describe("LearningResourceCard", () => { - const makeResource = factories.learningResources.resource - type SetupOptions = { - user?: Partial - props?: Partial - } - const setup = ({ user, props = {} }: SetupOptions = {}) => { - const { resource = makeResource(), variant = "column" } = props - const { view, location } = renderWithProviders( - , - { user }, - ) - return { resource, view, location } - } - - const labels = { - addToLearningPaths: "Add to Learning Path", - addToUserList: "Add to User List", - } - - test("Applies className to the resource card", () => { - const { view } = setup({ user: {}, props: { className: "test-class" } }) - expect(view.container.firstChild).toHaveClass("test-class") - }) - - test.each([ - { - user: { is_authenticated: true, is_learning_path_editor: false }, - expectAddToLearningPathButton: false, - expectAddToUserListButton: true, - }, - { - user: { is_authenticated: true, is_learning_path_editor: true }, - expectAddToLearningPathButton: true, - expectAddToUserListButton: true, - }, - { - user: { is_authenticated: false }, - expectAddToLearningPathButton: false, - expectAddToUserListButton: false, - }, - ])( - "Shows add to list buttons if and only if user is authenticated and has editing privileges", - async ({ - user, - expectAddToLearningPathButton, - expectAddToUserListButton, - }) => { - setup({ user }) - const addToLearningPathButton = screen.queryByRole("button", { - name: labels.addToLearningPaths, - }) - const addToUserListButton = screen.queryByRole("button", { - name: labels.addToUserList, - }) - expect(!!addToLearningPathButton).toBe(expectAddToLearningPathButton) - expect(!!addToUserListButton).toBe(expectAddToUserListButton) - }, - ) - - test("Clicking add to list button opens AddToListDialog", async () => { - const showModal = jest.mocked(NiceModal.show) - - const { resource } = setup({ - user: { is_learning_path_editor: true }, - }) - const addToLearningPathButton = screen.getByRole("button", { - name: labels.addToLearningPaths, - }) - const addToUserListButton = screen.getByRole("button", { - name: labels.addToUserList, - }) - - expect(showModal).not.toHaveBeenCalled() - await user.click(addToLearningPathButton) - expect(showModal).toHaveBeenLastCalledWith(AddToLearningPathDialog, { - resourceId: resource.id, - }) - await user.click(addToUserListButton) - expect(showModal).toHaveBeenLastCalledWith(AddToUserListDialog, { - resourceId: resource.id, - }) - }) - - test("Clicking card title opens resource drawer", async () => { - const { resource, location } = setup({ - user: { is_learning_path_editor: true }, - }) - const cardTitle = screen.getByRole("heading", { name: resource.title }) - await user.click(cardTitle) - expect( - new URLSearchParams(location.current.search).get( - RESOURCE_DRAWER_QUERY_PARAM, - ), - ).toBe(String(resource.id)) - }) -}) diff --git a/frontends/mit-open/src/page-components/LearningResourceCard/LearningResourceCard.tsx b/frontends/mit-open/src/page-components/LearningResourceCard/LearningResourceCard.tsx deleted file mode 100644 index 11b26ae985..0000000000 --- a/frontends/mit-open/src/page-components/LearningResourceCard/LearningResourceCard.tsx +++ /dev/null @@ -1,101 +0,0 @@ -/* - * TODO: This has been replaced by the ol-components LearningResourceCard - * It is still in use by the LearningPathDetailsPage -> ListDetails -> ItemsListing - * though can be removed (and adjacent LearningResourceCardTemplate) once - * the sorting functionality has been refactored across - */ -import React, { useCallback } from "react" -import * as NiceModal from "@ebay/nice-modal-react" - -import LearningResourceCardTemplate from "@/page-components/LearningResourceCardTemplate/LearningResourceCardTemplate" -import type { LearningResourceCardTemplateProps } from "@/page-components/LearningResourceCardTemplate/LearningResourceCardTemplate" -import { ActionButton, imgConfigs } from "ol-components" -import { - AddToLearningPathDialog, - AddToUserListDialog, -} from "../Dialogs/AddToListDialog" -import { LearningResource } from "api" -import { useUserMe } from "api/hooks/user" -import { useOpenLearningResourceDrawer } from "../LearningResourceDrawer/LearningResourceDrawer" - -import { RiMenuAddLine, RiBookmarkLine } from "@remixicon/react" - -type LearningResourceCardProps = Pick< - LearningResourceCardTemplateProps, - "variant" | "resource" | "className" | "sortable" | "suppressImage" -> - -/** - * Our standard LearningResourceCard component for MIT Open. - * - * This is mostly a wrapper around {@link LearningResourceCardTemplate }, which - * provides no meaningful user interactions on its own. This component connects - * {@link LearningResourceCardTemplate } to app features like the resource - * drawer and userlist dialogs. - * - - */ -const LearningResourceCard: React.FC = ({ - resource, - variant, - className, - sortable, - suppressImage, -}) => { - const showAddToLearningPathDialog = useCallback(() => { - NiceModal.show(AddToLearningPathDialog, { resourceId: resource.id }) - return - }, [resource]) - const showAddToUserListDialog = useCallback(() => { - NiceModal.show(AddToUserListDialog, { resourceId: resource.id }) - return - }, [resource]) - - const openLRDrawer = useOpenLearningResourceDrawer() - const { isLoading, data: user } = useUserMe() - - if (isLoading) { - return null - } - - return ( - openLRDrawer(r.id)} - footerActionSlot={ -
    - {user?.is_authenticated && user?.is_learning_path_editor && ( - - - - )} - {user?.is_authenticated && ( - - - - )} -
    - } - /> - ) -} - -export default LearningResourceCard -export type { LearningResourceCardProps } diff --git a/frontends/mit-open/src/page-components/LearningResourceCardTemplate/LearningResourceCardTemplate.test.tsx b/frontends/mit-open/src/page-components/LearningResourceCardTemplate/LearningResourceCardTemplate.test.tsx deleted file mode 100644 index fc121f569d..0000000000 --- a/frontends/mit-open/src/page-components/LearningResourceCardTemplate/LearningResourceCardTemplate.test.tsx +++ /dev/null @@ -1,167 +0,0 @@ -import React from "react" -import { render, screen } from "@testing-library/react" -import { assertInstanceOf, resourceThumbnailSrc } from "ol-utilities" -import LearningResourceCardTemplate from "./LearningResourceCardTemplate" -import { makeImgConfig } from "ol-utilities/test-utils/factories" -import { allowConsoleErrors } from "ol-test-utilities" -import user from "@testing-library/user-event" -import * as factories from "api/test-utils/factories" -import { ResourceTypeEnum } from "api" - -const factory = factories.learningResources - -describe("LearningResourceCard", () => { - it("renders title and cover image", () => { - const resource = factory.course() - const imgConfig = makeImgConfig() - render( - , - ) - const heading = screen.getByRole("heading", { name: resource.title }) - - const coverImg = screen.getByRole("img", { name: "" }) - assertInstanceOf(coverImg, HTMLImageElement) - expect(heading).toHaveAccessibleName(resource.title) - expect(coverImg.alt).toBe("") // Alert! This should be empty unless it is meaningful. - expect(coverImg.src).toBe(resourceThumbnailSrc(resource.image, imgConfig)) - }) - - it("does not show an image if and only if suppressImage is true", () => { - const resource = factory.course() - const imgConfig = makeImgConfig() - const { rerender } = render( - , - ) - const images = screen.queryAllByRole("img") - rerender( - , - ) - expect(screen.queryAllByRole("img").length).toBe(images.length - 1) - }) - - it("Calls onActivate when clicking title", async () => { - const resource = factory.course() - const imgConfig = makeImgConfig() - const onActivate = jest.fn() - render( - , - ) - - const heading = screen.getByRole("button", { name: resource.title }) - await user.click(heading) - expect(onActivate).toHaveBeenCalledWith(resource) - }) - - it.each([{ certification: false }, { certification: true }])( - "should render an icon if the object has a certificate", - ({ certification }) => { - const resource = factory.course({ - certification, - }) - const imgConfig = makeImgConfig() - - render( - , - ) - const certIcon = screen.queryByAltText("Receive a certificate", { - exact: false, - }) - expect(certIcon === null).not.toBe(certification) - }, - ) - - it("Should show an item count if the resource is a list", () => { - const resource = factory.learningPath() - const count = resource.learning_path?.item_count - const imgConfig = makeImgConfig() - - render( - , - ) - const itemText = count === 1 ? "1 item" : `${count} items` - const itemCount = screen.getByText(itemText) - expect(itemCount).toBeVisible() - }) - - it("Should NOT show an item count if the resource is NOT a list", () => { - const resource = factory.resource({ - title: "Not a list", - resource_type: ResourceTypeEnum.Course, - }) - const imgConfig = makeImgConfig() - - render( - , - ) - const itemCount = screen.queryByText("item", { exact: false }) - expect(itemCount).toBe(null) - }) - - it.each([ - { sortable: true, shows: "Shows" }, - { sortable: false, shows: "Does not show" }, - ])("$shows a drag handle when sortable is $sortable", ({ sortable }) => { - const resource = factory.resource() - const imgConfig = makeImgConfig() - render( - , - ) - - expect(!!screen.queryByTestId("CardDraggable")).toBe(sortable) - }) - - it.each([{ variant: "row" }, { variant: "column" }] as const)( - "Throws error if sortable & unsupported variant", - ({ variant }) => { - const resource = factory.resource() - const imgConfig = makeImgConfig() - const shouldThrow = () => { - render( - , - ) - } - allowConsoleErrors() - expect(shouldThrow).toThrow(/only supported for variant='row-reverse'/) - }, - ) -}) diff --git a/frontends/mit-open/src/page-components/LearningResourceCardTemplate/LearningResourceCardTemplate.tsx b/frontends/mit-open/src/page-components/LearningResourceCardTemplate/LearningResourceCardTemplate.tsx deleted file mode 100644 index 7fda5f83e6..0000000000 --- a/frontends/mit-open/src/page-components/LearningResourceCardTemplate/LearningResourceCardTemplate.tsx +++ /dev/null @@ -1,167 +0,0 @@ -import React, { useCallback } from "react" -import { ResourceTypeEnum } from "api" -import type { LearningResource } from "api" -import { Chip, styled } from "ol-components" -import { RiCalendarLine } from "@remixicon/react" -import { - formatDate, - pluralize, - getReadableResourceType, - findBestRun, - DEFAULT_RESOURCE_IMG, -} from "ol-utilities" -import type { EmbedlyConfig } from "ol-utilities" -import CardTemplate from "../CardTemplate/CardTemplate" - -type CardVariant = "column" | "row" | "row-reverse" -type OnActivateCard = (resource: R) => void -type LearningResourceCardTemplateProps< - R extends LearningResource = LearningResource, -> = { - /** - * Whether the course picture and info display as a column or row. - */ - variant: CardVariant - resource: R - sortable?: boolean - className?: string - /** - * Config used to generate embedly urls. - */ - imgConfig: EmbedlyConfig - onActivate?: OnActivateCard - /** - * Suppress the image. - */ - suppressImage?: boolean - footerActionSlot?: React.ReactNode -} - -const LIGHT_TEXT_COLOR = "#8c8c8c" -const SMALL_FONT_SIZE = 0.75 - -const CalendarChip = styled(Chip)({ - height: `${2.5 * SMALL_FONT_SIZE}em`, - fontSize: `${SMALL_FONT_SIZE}em`, - - ".MuiSvgIcon-root": { - height: `${1.25 * SMALL_FONT_SIZE}em`, - width: `${1.25 * SMALL_FONT_SIZE}em`, - }, -}) - -const ResourceFooterDetails: React.FC< - Pick -> = ({ resource }) => { - if (resource.resource_type === ResourceTypeEnum.LearningPath) { - const count = resource.learning_path.item_count - return ( - - {count} {pluralize("item", count)} - - ) - } - - const bestRun = findBestRun(resource.runs ?? []) - const startDate = bestRun?.start_date - - if (!startDate) return null - - const formattedDate = formatDate(startDate, "MMMM DD, YYYY") - - return ( - } - label={formattedDate} - /> - ) -} - -const OfferedByText = styled.span` - color: ${LIGHT_TEXT_COLOR}; - padding-right: 0.25em; -` - -const CardBody: React.FC< - Pick -> = ({ resource }) => { - const offerer = resource.offered_by?.name ?? null - return offerer ? ( -
    - Offered by – - {offerer} -
    - ) : null -} - -const TypeRow = styled.div` - display: flex; - flex-direction: row; - justify-content: space-between; - align-items: center; - min-height: 1.5em; /* ensure consistent height even if no certificate */ -` - -const CertificateIcon = styled.img` - height: 1.5em; -` -/** - * A card display for Learning Resources. Includes a title, image, and various - * metadata. - * - * This template does not provide any meaningful user interaction by itself, but - * does accept props to build user interaction (e.g., `onActivate` and - * `footerActionSlot`). - */ -const LearningResourceCardTemplate = ({ - variant, - resource, - imgConfig, - className, - onActivate, - footerActionSlot, - sortable, - suppressImage = false, -}: LearningResourceCardTemplateProps) => { - const handleActivate = useCallback( - () => onActivate?.(resource), - [resource, onActivate], - ) - - const imgUrl = resource.image?.url ?? DEFAULT_RESOURCE_IMG - const extraDetails = ( - - {getReadableResourceType(resource.resource_type)} - {resource.certification && ( - - )} - - ) - const body = - const footer = - - return ( - - ) -} - -export default LearningResourceCardTemplate -export { TypeRow } -export type { LearningResourceCardTemplateProps } diff --git a/frontends/mit-open/src/page-components/UserListCardTemplate/UserListCardTemplate.tsx b/frontends/mit-open/src/page-components/UserListCardTemplate/UserListCardTemplate.tsx index 4e6c2cdf10..91da95becc 100644 --- a/frontends/mit-open/src/page-components/UserListCardTemplate/UserListCardTemplate.tsx +++ b/frontends/mit-open/src/page-components/UserListCardTemplate/UserListCardTemplate.tsx @@ -2,7 +2,15 @@ import React, { useCallback } from "react" import CardTemplate from "../CardTemplate/CardTemplate" import { UserList } from "api" import { EmbedlyConfig, pluralize } from "ol-utilities" -import { TypeRow } from "../LearningResourceCardTemplate/LearningResourceCardTemplate" +import { styled } from "ol-components" + +const TypeRow = styled.div` + display: flex; + flex-direction: row; + justify-content: space-between; + align-items: center; + min-height: 1.5em; /* ensure consistent height even if no certificate */ +` type CardVariant = "column" | "row" | "row-reverse" type OnActivateCard = (userList: UserList) => void diff --git a/frontends/mit-open/src/pages/DashboardPage/UserListDetailsTab.tsx b/frontends/mit-open/src/pages/DashboardPage/UserListDetailsTab.tsx index 9e81126107..57007d463c 100644 --- a/frontends/mit-open/src/pages/DashboardPage/UserListDetailsTab.tsx +++ b/frontends/mit-open/src/pages/DashboardPage/UserListDetailsTab.tsx @@ -29,6 +29,7 @@ const UserListDetailsTab: React.FC = (props) => { isLoading={itemsQuery.isLoading} isFetching={itemsQuery.isFetching} handleEdit={() => manageListDialogs.upsertUserList(listQuery.data)} + condensed /> ) } diff --git a/frontends/mit-open/src/pages/SearchPage/SearchPage.tsx b/frontends/mit-open/src/pages/SearchPage/SearchPage.tsx index bb1cfed5be..4a87e5955c 100644 --- a/frontends/mit-open/src/pages/SearchPage/SearchPage.tsx +++ b/frontends/mit-open/src/pages/SearchPage/SearchPage.tsx @@ -26,6 +26,8 @@ const ColoredHeader = styled.div` height: 75px; } + position: relative; + z-index: -1; display: flex; align-items: center; background: #eb01a5; @@ -40,7 +42,7 @@ const BackgroundImage = styled.img` position: absolute; float: left; width: 35%; - top: 60px; + top: 0; left: 0; ${({ theme }) => theme.breakpoints.down("md")} { display: none; diff --git a/frontends/mit-open/src/test-utils/setupJest.ts b/frontends/mit-open/src/test-utils/setupJest.ts index 69013dcd68..f86c53d05a 100644 --- a/frontends/mit-open/src/test-utils/setupJest.ts +++ b/frontends/mit-open/src/test-utils/setupJest.ts @@ -15,15 +15,4 @@ jest.mock("axios", () => { } }) -jest.mock("@/page-components/LearningResourceCard/LearningResourceCard", () => { - const actual = jest.requireActual( - "@/page-components/LearningResourceCard/LearningResourceCard", - ) - return { - __esModule: true, - ...actual, - default: jest.fn(actual.default), - } -}) - window.scrollTo = jest.fn() diff --git a/frontends/ol-components/src/components/Card/ListCard.tsx b/frontends/ol-components/src/components/Card/ListCard.tsx index 40ff2a3bdc..f8217f8da1 100644 --- a/frontends/ol-components/src/components/Card/ListCard.tsx +++ b/frontends/ol-components/src/components/Card/ListCard.tsx @@ -5,9 +5,10 @@ import React, { ImgHTMLAttributes, isValidElement, } from "react" +import { Link } from "react-router-dom" import styled from "@emotion/styled" +import { RiDraggable } from "@remixicon/react" import { theme } from "../ThemeProvider/ThemeProvider" -import { Link } from "react-router-dom" import { Wrapper, containerStyles } from "./Card" import { TruncateText } from "../TruncateText/TruncateText" import { ActionButton, ActionButtonProps } from "../Button/Button" @@ -30,6 +31,11 @@ export const Container = styled.div` ${containerStyles} ` +export const DraggableContainer = styled.div` + ${containerStyles} + display: flex; +` + const Content = () => <> export const Body = styled.div` @@ -45,6 +51,30 @@ export const Body = styled.div` justify-content: space-between; ` +export const DragArea = styled.div` + margin: 16px -6px 16px 16px; + padding-right: 8px; + display: flex; + align-items: center; + justify-content: center; + border-right: 1px solid ${theme.custom.colors.lightGray2}; + + ${theme.breakpoints.down("md")} { + margin: 12px 0 12px 12px; + padding-right: 4px; + } + + svg { + fill: ${theme.custom.colors.silverGrayDark}; + width: 24px; + height: 24px; + ${theme.breakpoints.down("md")} { + width: 20px; + height: 20px; + } + } +` + const Image = styled.img` display: block; width: 236px; @@ -144,6 +174,7 @@ type CardProps = { children: ReactNode[] | ReactNode className?: string href?: string + draggable?: boolean } export type Card = FC & { Content: FC<{ children: ReactNode }> @@ -155,8 +186,12 @@ export type Card = FC & { Action: FC } -const ListCard: Card = ({ children, className, href }) => { - const _Container = href ? LinkContainer : Container +const ListCard: Card = ({ children, className, href, draggable }) => { + const _Container = draggable + ? DraggableContainer + : href + ? LinkContainer + : Container let content, imageProps, info, title, footer, actions @@ -181,6 +216,11 @@ const ListCard: Card = ({ children, className, href }) => { return ( <_Container to={href!}> + {draggable && ( + + + + )} {info} diff --git a/frontends/ol-components/src/components/Card/ListCardCondensed.tsx b/frontends/ol-components/src/components/Card/ListCardCondensed.tsx index 219fb64e0e..4943abfb1a 100644 --- a/frontends/ol-components/src/components/Card/ListCardCondensed.tsx +++ b/frontends/ol-components/src/components/Card/ListCardCondensed.tsx @@ -1,5 +1,6 @@ import React, { FC, ReactNode, Children, isValidElement } from "react" import styled from "@emotion/styled" +import { RiDraggable } from "@remixicon/react" import { theme } from "../ThemeProvider/ThemeProvider" import { Wrapper } from "./Card" import { TruncateText } from "../TruncateText/TruncateText" @@ -8,6 +9,8 @@ import { Body as BaseBody, LinkContainer, Container, + DraggableContainer, + DragArea as BaseDragArea, Info as BaseInfo, Title as BaseTitle, Footer, @@ -16,6 +19,15 @@ import { } from "./ListCard" import type { Card as BaseCard } from "./ListCard" +const DragArea = styled(BaseDragArea)` + padding-right: 4px; + margin-right: -4px; + ${theme.breakpoints.down("md")} { + margin: 12px -4px 12px 12px; + padding-right: 4px; + } +` + const Body = styled(BaseBody)` margin: 16px; ${theme.breakpoints.down("md")} { @@ -59,12 +71,17 @@ type CardProps = { children: ReactNode[] | ReactNode className?: string href?: string + draggable?: boolean } type Card = FC<CardProps> & Omit<BaseCard, "Image"> -const ListCardCondensed: Card = ({ children, className, href }) => { - const _Container = href ? LinkContainer : Container +const ListCardCondensed: Card = ({ children, className, href, draggable }) => { + const _Container = draggable + ? DraggableContainer + : href + ? LinkContainer + : Container let content, info, title, footer, actions @@ -88,6 +105,11 @@ const ListCardCondensed: Card = ({ children, className, href }) => { return ( <Wrapper className={className}> <_Container to={href!}> + {draggable && ( + <DragArea> + <RiDraggable /> + </DragArea> + )} <Body> <Info>{info}</Info> <Title> diff --git a/frontends/ol-components/src/components/LearningResourceCard/LearningResourceListCard.stories.tsx b/frontends/ol-components/src/components/LearningResourceCard/LearningResourceListCard.stories.tsx index 78898821d7..4414f8c681 100644 --- a/frontends/ol-components/src/components/LearningResourceCard/LearningResourceListCard.stories.tsx +++ b/frontends/ol-components/src/components/LearningResourceCard/LearningResourceListCard.stories.tsx @@ -57,6 +57,7 @@ const meta: Meta<typeof LearningResourceListCard> = { isLoading, onAddToLearningPathClick, onAddToUserListClick, + draggable, }) => ( <LearningResourceListCard resource={resource} @@ -64,6 +65,7 @@ const meta: Meta<typeof LearningResourceListCard> = { href={`/?resource=${resource?.id}`} onAddToLearningPathClick={onAddToLearningPathClick} onAddToUserListClick={onAddToUserListClick} + draggable={draggable} /> ), decorators: [withRouter], @@ -151,3 +153,13 @@ export const Loading: Story = { isLoading: true, }, } + +export const Draggable: Story = { + args: { + resource: makeResource({ + resource_type: ResourceTypeEnum.Course, + runs: [factories.learningResources.run()], + }), + draggable: true, + }, +} diff --git a/frontends/ol-components/src/components/LearningResourceCard/LearningResourceListCard.tsx b/frontends/ol-components/src/components/LearningResourceCard/LearningResourceListCard.tsx index 9c88bbdcd6..8935db3c9b 100644 --- a/frontends/ol-components/src/components/LearningResourceCard/LearningResourceListCard.tsx +++ b/frontends/ol-components/src/components/LearningResourceCard/LearningResourceListCard.tsx @@ -356,6 +356,7 @@ interface LearningResourceListCardProps { editMenu?: React.ReactNode | null inUserList?: boolean inLearningPath?: boolean + draggable?: boolean } type CardActionButtonProps = Pick< @@ -394,6 +395,7 @@ const LearningResourceListCard: React.FC<LearningResourceListCardProps> = ({ editMenu, inLearningPath, inUserList, + draggable, }) => { const isMobile = !useMuiBreakpointAtLeast("md") @@ -409,8 +411,9 @@ const LearningResourceListCard: React.FC<LearningResourceListCardProps> = ({ if (!resource) { return null } + return ( - <ListCard href={href} className={className}> + <ListCard href={href} className={className} draggable={draggable}> <ListCard.Image src={ resource.image?.url diff --git a/frontends/ol-components/src/components/LearningResourceCard/LearningResourceListCardCondensed.stories.tsx b/frontends/ol-components/src/components/LearningResourceCard/LearningResourceListCardCondensed.stories.tsx index 4ec396c2d6..d12b9dd671 100644 --- a/frontends/ol-components/src/components/LearningResourceCard/LearningResourceListCardCondensed.stories.tsx +++ b/frontends/ol-components/src/components/LearningResourceCard/LearningResourceListCardCondensed.stories.tsx @@ -57,6 +57,7 @@ const meta: Meta<typeof LearningResourceListCardCondensed> = { isLoading, onAddToLearningPathClick, onAddToUserListClick, + draggable, }) => ( <LearningResourceListCardCondensed resource={resource} @@ -64,6 +65,7 @@ const meta: Meta<typeof LearningResourceListCardCondensed> = { href={`/?resource=${resource?.id}`} onAddToLearningPathClick={onAddToLearningPathClick} onAddToUserListClick={onAddToUserListClick} + draggable={draggable} /> ), decorators: [withRouter], @@ -151,3 +153,13 @@ export const Loading: Story = { isLoading: true, }, } + +export const Draggable: Story = { + args: { + resource: makeResource({ + resource_type: ResourceTypeEnum.Course, + runs: [factories.learningResources.run()], + }), + draggable: true, + }, +} diff --git a/frontends/ol-components/src/components/LearningResourceCard/LearningResourceListCardCondensed.tsx b/frontends/ol-components/src/components/LearningResourceCard/LearningResourceListCardCondensed.tsx index efb37027cd..c057d27818 100644 --- a/frontends/ol-components/src/components/LearningResourceCard/LearningResourceListCardCondensed.tsx +++ b/frontends/ol-components/src/components/LearningResourceCard/LearningResourceListCardCondensed.tsx @@ -100,6 +100,7 @@ const LearningResourceListCardCondensed: React.FC< editMenu, inLearningPath, inUserList, + draggable, }) => { const isMobile = !useMuiBreakpointAtLeast("md") @@ -116,7 +117,7 @@ const LearningResourceListCardCondensed: React.FC< return null } return ( - <ListCardCondensed href={href} className={className}> + <ListCardCondensed href={href} className={className} draggable={draggable}> <ListCardCondensed.Info> <Info resource={resource} /> </ListCardCondensed.Info> diff --git a/frontends/ol-components/src/components/LearningResourceExpanded/InfoSection.tsx b/frontends/ol-components/src/components/LearningResourceExpanded/InfoSection.tsx index 6b4ab05398..7980779598 100644 --- a/frontends/ol-components/src/components/LearningResourceExpanded/InfoSection.tsx +++ b/frontends/ol-components/src/components/LearningResourceExpanded/InfoSection.tsx @@ -154,7 +154,7 @@ const INFO_ITEMS: InfoItemConfig = [ Icon: RiListOrdered2, selector: (resource: LearningResource) => { if (resource.resource_type === ResourceTypeEnum.Program) { - return resource.program?.courses?.length || null + return resource.program.course_count } return null }, diff --git a/frontends/ol-components/src/components/SortableList/SortableList.tsx b/frontends/ol-components/src/components/SortableList/SortableList.tsx index ecafe73562..57a59238ce 100644 --- a/frontends/ol-components/src/components/SortableList/SortableList.tsx +++ b/frontends/ol-components/src/components/SortableList/SortableList.tsx @@ -29,11 +29,10 @@ const DragStyles = styled.div` .ol-draggable { cursor: grab; } +` - .ol-dragging, - .ol-dragging * { - cursor: grabbing; - } +const DraggingContainer = styled.div` + cursor: grabbing; ` type SortableItemProps<I extends UniqueIdentifier = UniqueIdentifier> = { @@ -61,6 +60,7 @@ const SortableItem = <I extends UniqueIdentifier = UniqueIdentifier>( isDragging, setActivatorNodeRef, } = useSortable({ id: props.id, data: props.data, disabled: props.disabled }) + const style = { transform: CSS.Translate.toString(transform), transition, @@ -234,7 +234,7 @@ const SortableList = <I extends UniqueIdentifier = UniqueIdentifier>({ {children} </SortableContext> <DragOverlay dropAnimation={dropAnimationConfig}> - <div className="ol-dragging">{active && renderActive(active)}</div> + <DraggingContainer>{active && renderActive(active)}</DraggingContainer> </DragOverlay> </DndContext> ) diff --git a/learning_resources/models.py b/learning_resources/models.py index 7b35513420..69cb1b406b 100644 --- a/learning_resources/models.py +++ b/learning_resources/models.py @@ -1,11 +1,12 @@ """Models for learning resources and related entities""" from decimal import Decimal +from functools import cached_property 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, OuterRef, Q +from django.db.models import Count, JSONField, OuterRef, Prefetch, Q from django.db.models.functions import Lower from django.utils import timezone @@ -46,10 +47,10 @@ def annotate_channel_url(self): from channels.models import Channel return self.annotate( - _channel_url=( + channel_url=( Channel.objects.filter(topic_detail__topic=OuterRef("pk")) .annotate_channel_url() - .values_list("_channel_url", flat=True)[:1] + .values_list("channel_url", flat=True)[:1] ), ) @@ -73,7 +74,7 @@ def __str__(self): """Return the topic name.""" return self.name - @property + @cached_property def channel_url(self): """Return the topic's channel url""" if hasattr(self, "_channel_url"): @@ -97,10 +98,10 @@ def annotate_channel_url(self): from channels.models import Channel return self.annotate( - _channel_url=( + channel_url=( Channel.objects.filter(unit_detail__unit=OuterRef("pk")) .annotate_channel_url() - .values_list("_channel_url", flat=True)[:1] + .values_list("channel_url", flat=True)[:1] ), ) @@ -126,7 +127,7 @@ class LearningResourceOfferor(TimestampedModel): # This field name means "value proposition" value_prop = models.TextField(blank=True) - @property + @cached_property def channel_url(self): """Return the offeror's channel url""" if hasattr(self, "_channel_url"): @@ -210,34 +211,29 @@ def __str__(self): return self.full_name or f"{self.first_name} {self.last_name}" +def _map_resource_type_prefetch(name) -> Prefetch: + if name == LearningResourceType.program.name: + return Prefetch( + name, + queryset=Program.objects.annotate( + course_count=Count( + "learning_resource__children", + filter=Q( + learning_resource__children__relation_type=LearningResourceRelationTypes.PROGRAM_COURSES.value, + learning_resource__children__child__published=True, + ), + ) + ), + ) + return Prefetch(name) + + class LearningResource(TimestampedModel): """Core model for all learning resources""" - prefetches = [ - "topics", - "offered_by", - "departments", - "departments__school", - "content_tags", - "runs", - "runs__instructors", - "runs__image", - "children__child", - "children__child__runs", - "children__child__runs__instructors", - "children__child__departments", - "children__child__platform", - "children__child__topics", - "children__child__image", - "children__child__offered_by", - "children__child__content_tags", - *[f"children__child__{item.name}" for item in LearningResourceType], - ] - related_selects = [ "image", "platform", - *([item.name for item in LearningResourceType]), ] readable_id = models.CharField(max_length=512, null=False, blank=False) @@ -287,6 +283,28 @@ class LearningResource(TimestampedModel): professional = models.BooleanField(default=False) next_start_date = models.DateTimeField(null=True, blank=True, db_index=True) + @staticmethod + def get_prefetches(): + """Return the list of prefetches""" + return [ + Prefetch( + "topics", queryset=LearningResourceTopic.objects.annotate_channel_url() + ), + "offered_by", + "departments", + "departments__school", + "content_tags", + "runs", + "runs__instructors", + "runs__image", + *( + [ + _map_resource_type_prefetch(item.name) + for item in LearningResourceType + ] + ), + ] + @property def audience(self) -> str | None: """Returns the audience for the learning resource""" @@ -410,6 +428,14 @@ def courses(self): """Get the associated resources (should all be courses)""" return self.learning_resource.children + @cached_property + def course_count(self): + """Return the number of courses in the program""" + return self.learning_resource.children.filter( + relation_type=LearningResourceRelationTypes.PROGRAM_COURSES, + child__published=True, + ).count() + class LearningPath(TimestampedModel): """ diff --git a/learning_resources/serializers.py b/learning_resources/serializers.py index ec028c207a..68ed8afe0d 100644 --- a/learning_resources/serializers.py +++ b/learning_resources/serializers.py @@ -7,7 +7,6 @@ from django.contrib.auth.models import User from django.db import transaction from django.db.models import F, Max -from drf_spectacular.helpers import lazy_serializer from drf_spectacular.utils import extend_schema_field from rest_framework import serializers from rest_framework.exceptions import ValidationError @@ -22,7 +21,7 @@ LevelType, ) from learning_resources.etl.loaders import update_index -from learning_resources.models import LearningResource, LearningResourceRelationship +from learning_resources.models import LearningResourceRelationship from main.serializers import COMMON_IGNORED_FIELDS, WriteableSerializerMethodField log = logging.getLogger(__name__) @@ -43,7 +42,7 @@ class LearningResourceTopicSerializer(serializers.ModelSerializer): Serializer for LearningResourceTopic model """ - channel_url = serializers.CharField() + channel_url = serializers.CharField(read_only=True) class Meta: """Meta options for the serializer.""" @@ -279,34 +278,11 @@ class CourseNumberSerializer(serializers.Serializer): class ProgramSerializer(serializers.ModelSerializer): """Serializer for the Program model""" - courses = serializers.SerializerMethodField() - - @extend_schema_field( - lazy_serializer("learning_resources.serializers.CourseResourceSerializer")( - many=True, allow_null=True - ) - ) - def get_courses(self, obj): - """Get the learning resource courses for a program""" - ids = ( - LearningResourceRelationship.objects.filter( - parent_id=obj.learning_resource.id, child__published=True - ) - .values_list("child_id", flat=True) - .distinct() - ) - return CourseResourceSerializer( - list( - LearningResource.objects.filter(id__in=ids) - .select_related(*LearningResource.related_selects) - .prefetch_related(*LearningResource.prefetches) - .order_by("next_start_date", "id") - ), - many=True, - ).data + course_count = serializers.IntegerField(read_only=True) class Meta: model = models.Program + include = ("course_count",) exclude = ("learning_resource", *COMMON_IGNORED_FIELDS) diff --git a/learning_resources/serializers_test.py b/learning_resources/serializers_test.py index e12adfe052..000593b718 100644 --- a/learning_resources/serializers_test.py +++ b/learning_resources/serializers_test.py @@ -64,15 +64,7 @@ def test_serialize_program_to_json(): serializer = serializers.ProgramSerializer(instance=program) assert_json_equal( serializer.data, - { - "courses": [ - # this is currently messy because program.courses is a list of LearningResourceRelationships - serializers.CourseResourceSerializer(instance=course_rel.child).data - for course_rel in program.courses.filter( - child__published=True - ).order_by("child__next_start_date", "child__id") - ] - }, + {"course_count": program.courses.filter(child__published=True).count()}, ) diff --git a/learning_resources/views.py b/learning_resources/views.py index bab8154b11..6043b88ce0 100644 --- a/learning_resources/views.py +++ b/learning_resources/views.py @@ -146,7 +146,7 @@ def _get_base_queryset(self, resource_type: str | None = None) -> QuerySet: if resource_type: lr_query = lr_query.filter(resource_type=resource_type) lr_query = lr_query.select_related(*LearningResource.related_selects) - return lr_query.prefetch_related(*LearningResource.prefetches).distinct() + return lr_query.prefetch_related(*LearningResource.get_prefetches()).distinct() def get_queryset(self) -> QuerySet: """ diff --git a/learning_resources/views_test.py b/learning_resources/views_test.py index 9dd1e7f1cc..b63127ac70 100644 --- a/learning_resources/views_test.py +++ b/learning_resources/views_test.py @@ -190,26 +190,17 @@ def test_program_endpoint(client, url, params): @pytest.mark.parametrize( "url", ["lr:v1:programs_api-detail", "lr:v1:learning_resources_api-detail"] ) -def test_program_detail_endpoint(client, url): +def test_program_detail_endpoint(client, django_assert_num_queries, url): """Test program endpoint""" program = ProgramFactory.create() - resp = client.get(reverse(url, args=[program.learning_resource.id])) + with django_assert_num_queries(21): + resp = client.get(reverse(url, args=[program.learning_resource.id])) assert resp.data.get("title") == program.learning_resource.title assert resp.data.get("resource_type") == LearningResourceType.program.name - response_courses = sorted(resp.data["program"]["courses"], key=lambda i: i["id"]) - - courses = sorted( - [relation.child for relation in program.courses.all()], key=lambda lr: lr.id + assert ( + resp.data["program"]["course_count"] + == program.learning_resource.children.count() ) - assert len(response_courses) == len(courses) - assert [course.id for course in courses] == [ - course["id"] for course in response_courses - ] - for idx, course in enumerate(courses): - assert course.id == response_courses[idx]["id"] - assert ( - response_courses[idx]["resource_type"] == LearningResourceType.course.name - ) def test_list_resources_endpoint(client): @@ -244,7 +235,7 @@ def test_no_excess_queries(mocker, django_assert_num_queries, course_count): CourseFactory.create_batch(course_count) - with django_assert_num_queries(10): + with django_assert_num_queries(16): view = CourseViewSet(request=mocker.Mock(query_params=[])) results = view.get_queryset().all() assert len(results) == course_count diff --git a/learning_resources_search/serializers.py b/learning_resources_search/serializers.py index 9aa6281c91..2780267745 100644 --- a/learning_resources_search/serializers.py +++ b/learning_resources_search/serializers.py @@ -606,7 +606,7 @@ def serialize_bulk_learning_resources(ids): """ for learning_resource in ( LearningResource.objects.select_related(*LearningResource.related_selects) - .prefetch_related(*LearningResource.prefetches) + .prefetch_related(*LearningResource.get_prefetches()) .filter(id__in=ids) ): yield serialize_learning_resource_for_bulk(learning_resource) diff --git a/main/settings.py b/main/settings.py index 583a5f6b7d..8e6e95957e 100644 --- a/main/settings.py +++ b/main/settings.py @@ -33,7 +33,7 @@ from main.settings_pluggy import * # noqa: F403 from openapi.settings_spectacular import open_spectacular_settings -VERSION = "0.13.21" +VERSION = "0.13.22" log = logging.getLogger() diff --git a/openapi/specs/v0.yaml b/openapi/specs/v0.yaml index 8242ba4f42..8f16c5bcc3 100644 --- a/openapi/specs/v0.yaml +++ b/openapi/specs/v0.yaml @@ -1606,6 +1606,7 @@ components: nullable: true channel_url: type: string + readOnly: true required: - channel_url - id diff --git a/openapi/specs/v1.yaml b/openapi/specs/v1.yaml index 23361fa29f..a3b7e6440f 100644 --- a/openapi/specs/v1.yaml +++ b/openapi/specs/v1.yaml @@ -8664,6 +8664,7 @@ components: nullable: true channel_url: type: string + readOnly: true required: - channel_url - id @@ -10178,14 +10179,11 @@ components: type: object description: Serializer for the Program model properties: - courses: - type: array - items: - $ref: '#/components/schemas/CourseResource' - nullable: true + course_count: + type: integer readOnly: true required: - - courses + - course_count ProgramCertificate: type: object description: Serializer for Program Certificates