From c5e0c9c76401a5256cd8d0ec62b715a7cdd41758 Mon Sep 17 00:00:00 2001 From: "renovate[bot]" <29139614+renovate[bot]@users.noreply.github.com> Date: Wed, 10 Jul 2024 16:44:02 +0000 Subject: [PATCH 01/11] Update dependency Django to v4.2.14 (#1240) Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> --- poetry.lock | 8 ++++---- pyproject.toml | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/poetry.lock b/poetry.lock index 976bf68275..3f1e9d7a5c 100644 --- a/poetry.lock +++ b/poetry.lock @@ -853,13 +853,13 @@ static3 = "*" [[package]] name = "django" -version = "4.2.13" +version = "4.2.14" description = "A high-level Python web framework that encourages rapid development and clean, pragmatic design." optional = false python-versions = ">=3.8" files = [ - {file = "Django-4.2.13-py3-none-any.whl", hash = "sha256:a17fcba2aad3fc7d46fdb23215095dbbd64e6174bf4589171e732b18b07e426a"}, - {file = "Django-4.2.13.tar.gz", hash = "sha256:837e3cf1f6c31347a1396a3f6b65688f2b4bb4a11c580dcb628b5afe527b68a5"}, + {file = "Django-4.2.14-py3-none-any.whl", hash = "sha256:3ec32bc2c616ab02834b9cac93143a7dc1cdcd5b822d78ac95fc20a38c534240"}, + {file = "Django-4.2.14.tar.gz", hash = "sha256:fc6919875a6226c7ffcae1a7d51e0f2ceaf6f160393180818f6c95f51b1e7b96"}, ] [package.dependencies] @@ -4042,4 +4042,4 @@ test = ["big-O", "importlib-resources", "jaraco.functools", "jaraco.itertools", [metadata] lock-version = "2.0" python-versions = "3.12.4" -content-hash = "3f6b3b66c4a792b7a7b818395f7d93ba8fb9787b3e31dd7027fc0d89ca2f6563" +content-hash = "1d27a7fce358f3b0677a4005f627d96c3f2c001ea81a56ca3ea9a50504393f77" diff --git a/pyproject.toml b/pyproject.toml index 74ba1bb515..1334aa7c22 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -20,7 +20,7 @@ cffi = "^1.15.1" cryptography = "^42.0.0" dj-database-url = "^2.0.0" dj-static = "^0.0.6" -Django = "4.2.13" +Django = "4.2.14" django-anymail = {extras = ["mailgun"], version = "^10.0"} django-bitfield = "^2.2.0" django-cache-memoize = "^0.2.0" From 8db7e80b671ae23742d12da27de38e287636182b Mon Sep 17 00:00:00 2001 From: "renovate[bot]" <29139614+renovate[bot]@users.noreply.github.com> Date: Wed, 10 Jul 2024 20:21:46 +0000 Subject: [PATCH 02/11] Update dependency ruff to v0.5.1 (#1241) Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> --- poetry.lock | 40 ++++++++++++++++++++-------------------- pyproject.toml | 2 +- 2 files changed, 21 insertions(+), 21 deletions(-) diff --git a/poetry.lock b/poetry.lock index 3f1e9d7a5c..20b7a964bb 100644 --- a/poetry.lock +++ b/poetry.lock @@ -3362,29 +3362,29 @@ files = [ [[package]] name = "ruff" -version = "0.5.0" +version = "0.5.1" description = "An extremely fast Python linter and code formatter, written in Rust." optional = false python-versions = ">=3.7" files = [ - {file = "ruff-0.5.0-py3-none-linux_armv6l.whl", hash = "sha256:ee770ea8ab38918f34e7560a597cc0a8c9a193aaa01bfbd879ef43cb06bd9c4c"}, - {file = "ruff-0.5.0-py3-none-macosx_10_12_x86_64.whl", hash = "sha256:38f3b8327b3cb43474559d435f5fa65dacf723351c159ed0dc567f7ab735d1b6"}, - {file = "ruff-0.5.0-py3-none-macosx_11_0_arm64.whl", hash = "sha256:7594f8df5404a5c5c8f64b8311169879f6cf42142da644c7e0ba3c3f14130370"}, - {file = "ruff-0.5.0-py3-none-manylinux_2_17_aarch64.manylinux2014_aarch64.whl", hash = "sha256:adc7012d6ec85032bc4e9065110df205752d64010bed5f958d25dbee9ce35de3"}, - {file = "ruff-0.5.0-py3-none-manylinux_2_17_armv7l.manylinux2014_armv7l.whl", hash = "sha256:d505fb93b0fabef974b168d9b27c3960714d2ecda24b6ffa6a87ac432905ea38"}, - {file = "ruff-0.5.0-py3-none-manylinux_2_17_i686.manylinux2014_i686.whl", hash = "sha256:9dc5cfd3558f14513ed0d5b70ce531e28ea81a8a3b1b07f0f48421a3d9e7d80a"}, - {file = "ruff-0.5.0-py3-none-manylinux_2_17_ppc64.manylinux2014_ppc64.whl", hash = "sha256:db3ca35265de239a1176d56a464b51557fce41095c37d6c406e658cf80bbb362"}, - {file = "ruff-0.5.0-py3-none-manylinux_2_17_ppc64le.manylinux2014_ppc64le.whl", hash = "sha256:b1a321c4f68809fddd9b282fab6a8d8db796b270fff44722589a8b946925a2a8"}, - {file = "ruff-0.5.0-py3-none-manylinux_2_17_s390x.manylinux2014_s390x.whl", hash = "sha256:2c4dfcd8d34b143916994b3876b63d53f56724c03f8c1a33a253b7b1e6bf2a7d"}, - {file = "ruff-0.5.0-py3-none-manylinux_2_17_x86_64.manylinux2014_x86_64.whl", hash = "sha256:81e5facfc9f4a674c6a78c64d38becfbd5e4f739c31fcd9ce44c849f1fad9e4c"}, - {file = "ruff-0.5.0-py3-none-musllinux_1_2_aarch64.whl", hash = "sha256:e589e27971c2a3efff3fadafb16e5aef7ff93250f0134ec4b52052b673cf988d"}, - {file = "ruff-0.5.0-py3-none-musllinux_1_2_armv7l.whl", hash = "sha256:d2ffbc3715a52b037bcb0f6ff524a9367f642cdc5817944f6af5479bbb2eb50e"}, - {file = "ruff-0.5.0-py3-none-musllinux_1_2_i686.whl", hash = "sha256:cd096e23c6a4f9c819525a437fa0a99d1c67a1b6bb30948d46f33afbc53596cf"}, - {file = "ruff-0.5.0-py3-none-musllinux_1_2_x86_64.whl", hash = "sha256:46e193b36f2255729ad34a49c9a997d506e58f08555366b2108783b3064a0e1e"}, - {file = "ruff-0.5.0-py3-none-win32.whl", hash = "sha256:49141d267100f5ceff541b4e06552e98527870eafa1acc9dec9139c9ec5af64c"}, - {file = "ruff-0.5.0-py3-none-win_amd64.whl", hash = "sha256:e9118f60091047444c1b90952736ee7b1792910cab56e9b9a9ac20af94cd0440"}, - {file = "ruff-0.5.0-py3-none-win_arm64.whl", hash = "sha256:ed5c4df5c1fb4518abcb57725b576659542bdbe93366f4f329e8f398c4b71178"}, - {file = "ruff-0.5.0.tar.gz", hash = "sha256:eb641b5873492cf9bd45bc9c5ae5320648218e04386a5f0c264ad6ccce8226a1"}, + {file = "ruff-0.5.1-py3-none-linux_armv6l.whl", hash = "sha256:6ecf968fcf94d942d42b700af18ede94b07521bd188aaf2cd7bc898dd8cb63b6"}, + {file = "ruff-0.5.1-py3-none-macosx_10_12_x86_64.whl", hash = "sha256:204fb0a472f00f2e6280a7c8c7c066e11e20e23a37557d63045bf27a616ba61c"}, + {file = "ruff-0.5.1-py3-none-macosx_11_0_arm64.whl", hash = "sha256:d235968460e8758d1e1297e1de59a38d94102f60cafb4d5382033c324404ee9d"}, + {file = "ruff-0.5.1-py3-none-manylinux_2_17_aarch64.manylinux2014_aarch64.whl", hash = "sha256:38beace10b8d5f9b6bdc91619310af6d63dd2019f3fb2d17a2da26360d7962fa"}, + {file = "ruff-0.5.1-py3-none-manylinux_2_17_armv7l.manylinux2014_armv7l.whl", hash = "sha256:5e478d2f09cf06add143cf8c4540ef77b6599191e0c50ed976582f06e588c994"}, + {file = "ruff-0.5.1-py3-none-manylinux_2_17_i686.manylinux2014_i686.whl", hash = "sha256:f0368d765eec8247b8550251c49ebb20554cc4e812f383ff9f5bf0d5d94190b0"}, + {file = "ruff-0.5.1-py3-none-manylinux_2_17_ppc64.manylinux2014_ppc64.whl", hash = "sha256:3a9a9a1b582e37669b0138b7c1d9d60b9edac880b80eb2baba6d0e566bdeca4d"}, + {file = "ruff-0.5.1-py3-none-manylinux_2_17_ppc64le.manylinux2014_ppc64le.whl", hash = "sha256:bdd9f723e16003623423affabcc0a807a66552ee6a29f90eddad87a40c750b78"}, + {file = "ruff-0.5.1-py3-none-manylinux_2_17_s390x.manylinux2014_s390x.whl", hash = "sha256:be9fd62c1e99539da05fcdc1e90d20f74aec1b7a1613463ed77870057cd6bd96"}, + {file = "ruff-0.5.1-py3-none-manylinux_2_17_x86_64.manylinux2014_x86_64.whl", hash = "sha256:e216fc75a80ea1fbd96af94a6233d90190d5b65cc3d5dfacf2bd48c3e067d3e1"}, + {file = "ruff-0.5.1-py3-none-musllinux_1_2_aarch64.whl", hash = "sha256:c4c2112e9883a40967827d5c24803525145e7dab315497fae149764979ac7929"}, + {file = "ruff-0.5.1-py3-none-musllinux_1_2_armv7l.whl", hash = "sha256:dfaf11c8a116394da3b65cd4b36de30d8552fa45b8119b9ef5ca6638ab964fa3"}, + {file = "ruff-0.5.1-py3-none-musllinux_1_2_i686.whl", hash = "sha256:d7ceb9b2fe700ee09a0c6b192c5ef03c56eb82a0514218d8ff700f6ade004108"}, + {file = "ruff-0.5.1-py3-none-musllinux_1_2_x86_64.whl", hash = "sha256:bac6288e82f6296f82ed5285f597713acb2a6ae26618ffc6b429c597b392535c"}, + {file = "ruff-0.5.1-py3-none-win32.whl", hash = "sha256:5c441d9c24ec09e1cb190a04535c5379b36b73c4bc20aa180c54812c27d1cca4"}, + {file = "ruff-0.5.1-py3-none-win_amd64.whl", hash = "sha256:b1789bf2cd3d1b5a7d38397cac1398ddf3ad7f73f4de01b1e913e2abc7dfc51d"}, + {file = "ruff-0.5.1-py3-none-win_arm64.whl", hash = "sha256:2875b7596a740cbbd492f32d24be73e545a4ce0a3daf51e4f4e609962bfd3cd2"}, + {file = "ruff-0.5.1.tar.gz", hash = "sha256:3164488aebd89b1745b47fd00604fb4358d774465f20d1fcd907f9c0fc1b0655"}, ] [[package]] @@ -4042,4 +4042,4 @@ test = ["big-O", "importlib-resources", "jaraco.functools", "jaraco.itertools", [metadata] lock-version = "2.0" python-versions = "3.12.4" -content-hash = "1d27a7fce358f3b0677a4005f627d96c3f2c001ea81a56ca3ea9a50504393f77" +content-hash = "63a09fdcd2006b42865926ecfdf6ec10351eb7472359887f2aed5dd19f63f78f" diff --git a/pyproject.toml b/pyproject.toml index 1334aa7c22..2ca7c8732f 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -78,7 +78,7 @@ django-scim2 = "^0.19.1" django-oauth-toolkit = "^2.3.0" youtube-transcript-api = "^0.6.2" posthog = "^3.5.0" -ruff = "0.5.0" +ruff = "0.5.1" [tool.poetry.group.dev.dependencies] bpython = "^0.24" From b789b815c7ac4daffccdc722783aa9cad9d372fb Mon Sep 17 00:00:00 2001 From: Chris Chudzicki Date: Thu, 11 Jul 2024 09:31:48 -0400 Subject: [PATCH 03/11] use main not "$default-branch" (#1249) --- .github/workflows/publish-pages.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/publish-pages.yml b/.github/workflows/publish-pages.yml index 4e9d044a61..1a5961a74b 100644 --- a/.github/workflows/publish-pages.yml +++ b/.github/workflows/publish-pages.yml @@ -1,7 +1,7 @@ on: # Runs on pushes targeting the default branch push: - branches: [$default-branch] + branches: [main] # Allows you to run this workflow manually from the Actions tab workflow_dispatch: From ed7dd68e86ab1aef8ee5118584e1cfd9018c664c Mon Sep 17 00:00:00 2001 From: Carey P Gumaer Date: Thu, 11 Jul 2024 09:37:39 -0400 Subject: [PATCH 04/11] adjust / refactor channel detail header (#1234) * add DefaultChannelSkeleton and use it when the channel type is not unit * use separate templates for units and other channel types * remove usage of BannerPage * align styles * more fine tuning of the styles * fix tests * remove unnecessary page wrappers * consolidate breadcrumb target info * de-flake test * move consolidated elements to ChannelPageSkeleton * consolidate more elements * flaky test fix #2 * rename Skeleton -> Template * rename subHeader -> subheader * add new banner stories * fix issue with wrapping breadcrumbs on mobile --- .../pages/ChannelPage/ChannelPage.test.tsx | 58 +++- .../src/pages/ChannelPage/ChannelPage.tsx | 14 +- .../pages/ChannelPage/ChannelPageSkeleton.tsx | 327 ------------------ .../pages/ChannelPage/ChannelPageTemplate.tsx | 95 +++++ .../pages/ChannelPage/ChannelSearch.test.tsx | 2 +- .../ChannelPage/DefaultChannelTemplate.tsx | 165 +++++++++ .../src/pages/ChannelPage/EditChannelPage.tsx | 6 +- .../pages/ChannelPage/UnitChannelTemplate.tsx | 184 ++++++++++ .../DepartmentListingPage.tsx | 4 +- .../TopicListingPage/TopicsListingPage.tsx | 4 +- .../UnitsListingPage/UnitsListingPage.tsx | 4 +- .../src/components/Banner/Banner.stories.tsx | 83 ++++- .../src/components/Banner/Banner.tsx | 127 +++++-- .../Breadcrumbs/Breadcrumbs.test.tsx | 2 +- .../components/Breadcrumbs/Breadcrumbs.tsx | 23 +- 15 files changed, 700 insertions(+), 398 deletions(-) delete mode 100644 frontends/mit-open/src/pages/ChannelPage/ChannelPageSkeleton.tsx create mode 100644 frontends/mit-open/src/pages/ChannelPage/ChannelPageTemplate.tsx create mode 100644 frontends/mit-open/src/pages/ChannelPage/DefaultChannelTemplate.tsx create mode 100644 frontends/mit-open/src/pages/ChannelPage/UnitChannelTemplate.tsx diff --git a/frontends/mit-open/src/pages/ChannelPage/ChannelPage.test.tsx b/frontends/mit-open/src/pages/ChannelPage/ChannelPage.test.tsx index 55c0606839..842d586e2d 100644 --- a/frontends/mit-open/src/pages/ChannelPage/ChannelPage.test.tsx +++ b/frontends/mit-open/src/pages/ChannelPage/ChannelPage.test.tsx @@ -101,17 +101,43 @@ const setupApis = ( describe("ChannelPage", () => { it("Displays the channel title, banner, and avatar", async () => { - const { channel } = setupApis() + const { channel } = setupApis({ + search_filter: "offered_by=ocw", + channel_type: "unit", + }) renderTestApp({ url: `/c/${channel.channel_type}/${channel.name}` }) - - const title = await screen.findAllByText(channel.title) - const header = title[0].closest("header") + const findTitle = (titles: HTMLElement[]) => { + return titles[ + titles.findIndex( + (title: HTMLElement) => + title.textContent === channel.title || + title.textContent === channel.configuration.heading, + ) + ] + } + await waitFor(() => { + const titles = screen.getAllByRole("heading") + const title = findTitle(titles) + expect(title).toBeInTheDocument() + }) + const titles = screen.getAllByRole("heading") + const title = findTitle(titles) + expect(title).toBeInTheDocument() + const header = title.closest("header") assertInstanceOf(header, HTMLElement) const images = within(header).getAllByRole("img") as HTMLImageElement[] const headerStyles = getComputedStyle(header) - expect(headerStyles.backgroundImage).toContain( - channel.configuration.banner_background, - ) + if (channel.channel_type !== "unit") { + /* + * unit channels are filtered out from this assertion + * because they wrap the background image in a linear-gradient, + * which causes react testing library to not render the background-image + * property at all + */ + expect(headerStyles.backgroundImage).toContain( + channel.configuration.banner_background, + ) + } expect(images[0].src).toContain(channel.configuration.logo) }) it("Displays a featured carousel if the channel type is 'unit'", async () => { @@ -158,7 +184,7 @@ describe("ChannelPage", () => { "platform=ocw&platform=mitxonline&department=8&department=9", }) renderTestApp({ url: `/c/${channel.channel_type}/${channel.name}` }) - await screen.findByText(channel.title) + await screen.findAllByText(channel.title) const expectedProps = expect.objectContaining({ constantSearchParams: { platform: ["ocw", "mitxonline"], @@ -176,7 +202,7 @@ describe("ChannelPage", () => { const { channel } = setupApis() channel.search_filter = undefined renderTestApp({ url: `/c/${channel.channel_type}/${channel.name}` }) - await screen.findByText(channel.title) + await screen.findAllByText(channel.title) expect(mockedChannelSearch).toHaveBeenCalledTimes(0) }) @@ -185,17 +211,17 @@ describe("ChannelPage", () => { const { channel } = setupApis() channel.search_filter = undefined renderTestApp({ url: `/c/${channel.channel_type}/${channel.name}` }) - await screen.findByText(channel.title) + await screen.findAllByText(channel.title) await waitFor(() => { - expect( - screen.getByText(channel.configuration.sub_heading), - ).toBeInTheDocument() + screen.getAllByText(channel.configuration.sub_heading).forEach((el) => { + expect(el).toBeInTheDocument() + }) }) await waitFor(() => { - expect( - screen.getByText(channel.configuration.heading), - ).toBeInTheDocument() + screen.getAllByText(channel.configuration.heading).forEach((el) => { + expect(el).toBeInTheDocument() + }) }) }) diff --git a/frontends/mit-open/src/pages/ChannelPage/ChannelPage.tsx b/frontends/mit-open/src/pages/ChannelPage/ChannelPage.tsx index 602382ce62..953f6f4329 100644 --- a/frontends/mit-open/src/pages/ChannelPage/ChannelPage.tsx +++ b/frontends/mit-open/src/pages/ChannelPage/ChannelPage.tsx @@ -1,12 +1,12 @@ import React from "react" import { useParams } from "react-router" -import ChannelPageSkeleton from "./ChannelPageSkeleton" +import { ChannelPageTemplate } from "./ChannelPageTemplate" import { useChannelDetail } from "api/hooks/channels" import FieldSearch from "./ChannelSearch" -import type { - Facets, - FacetKey, - BooleanFacets, +import { + type Facets, + type FacetKey, + type BooleanFacets, } from "@mitodl/course-search-utils" import { ChannelTypeEnum } from "api/v0" import TestimonialDisplay from "@/page-components/TestimonialDisplay/TestimonialDisplay" @@ -41,7 +41,7 @@ const ChannelPage: React.FC = () => { return ( name && channelType && ( - +

{channelQuery.data?.public_description}

{channelType === "unit" ? ( @@ -52,7 +52,7 @@ const ChannelPage: React.FC = () => { channelType={channelType} /> )} - + ) ) } diff --git a/frontends/mit-open/src/pages/ChannelPage/ChannelPageSkeleton.tsx b/frontends/mit-open/src/pages/ChannelPage/ChannelPageSkeleton.tsx deleted file mode 100644 index 2971c50264..0000000000 --- a/frontends/mit-open/src/pages/ChannelPage/ChannelPageSkeleton.tsx +++ /dev/null @@ -1,327 +0,0 @@ -import React, { useMemo } from "react" -import { Link } from "react-router-dom" -import * as routes from "../../common/urls" -import { - BannerPage, - styled, - Container, - Typography, - Box, - Breadcrumbs, -} from "ol-components" -import { MetaTags } from "ol-utilities" -import { SearchSubscriptionToggle } from "@/page-components/SearchSubscriptionToggle/SearchSubscriptionToggle" -import { ChannelDetails } from "@/page-components/ChannelDetails/ChannelDetails" -import { useChannelDetail } from "api/hooks/channels" -import ChannelMenu from "@/components/ChannelMenu/ChannelMenu" -import ChannelAvatar from "@/components/ChannelAvatar/ChannelAvatar" -import ResourceCarousel, { - ResourceCarouselProps, -} from "@/page-components/ResourceCarousel/ResourceCarousel" -import { SourceTypeEnum } from "api" -import { getSearchParamMap } from "@/common/utils" -import { DEPARTMENTS, HOME, TOPICS, UNITS } from "../../common/urls" - -export const ChannelTitleRow = styled.div` - display: flex; - flex-direction: row; - align-items: center; - - h1 a { - &:hover { - text-decoration: none; - } - } -` - -const FeaturedCoursesCarousel = styled(ResourceCarousel)(({ theme }) => ({ - margin: "80px 0", - [theme.breakpoints.down("sm")]: { - marginTop: "32px", - marginBottom: "32px", - }, -})) -export const ChannelControls = styled.div` - position: relative; - min-height: 38px; - display: flex; -` - -interface ChannelSkeletonProps { - children: React.ReactNode - channelType: string - name: string -} -const NAV_PATH: { [key: string]: { href: string; label: string } } = { - topic: { - href: TOPICS, - label: "Browse by Topic", - }, - department: { - href: DEPARTMENTS, - label: "Browse by Academic Department", - }, - unit: { - href: UNITS, - label: "MIT Units", - }, - pathway: { - href: "", - label: "Pathways", - }, -} - -/** - * Common structure for channel-oriented pages. - * - * Renders the channel title and avatar in a banner. - */ -const ChannelSkeletonProps: React.FC = ({ - children, - channelType, - name, -}) => { - const channel = useChannelDetail(String(channelType), String(name)) - const urlParams = new URLSearchParams(channel.data?.search_filter) - const displayConfiguration = channel.data?.configuration - - const urlParamMap: Record = useMemo(() => { - const urlParams = new URLSearchParams(channel.data?.search_filter) - return getSearchParamMap(urlParams) - }, [channel]) - - const FEATURED_RESOURCES_CAROUSEL: ResourceCarouselProps["config"] = [ - { - cardProps: { size: "medium" }, - data: { - type: "lr_featured", - params: { limit: 12, ...urlParamMap }, - }, - label: undefined, - }, - ] - - return ( - <> - - - - - {channel.data && ( - - - ({ - flexGrow: 1, - flexShrink: 0, - order: 1, - py: "24px", - - [theme.breakpoints.down("md")]: { - py: 0, - pb: "8px", - }, - [theme.breakpoints.down("sm")]: { - width: "100%", - }, - })} - > - {displayConfiguration?.logo ? ( - - ) : ( - - - {channel.data.title} - - - )} - - {displayConfiguration.heading ? ( - - - {displayConfiguration.heading} - - - ) : ( - <> - )} - - - {displayConfiguration.sub_heading} - - - {channelType === "unit" ? ( - - - {channel.data?.search_filter ? ( - - ) : null} - {channel.data?.is_moderator ? ( - - ) : null} - - - ) : null} - - {channelType === "unit" ? ( - - - - ) : ( - - - {channel.data?.search_filter ? ( - - ) : null} - {channel.data?.is_moderator ? ( - - ) : null} - - - )} - - )} - - - } - > - {channelType === "unit" ? ( - - - - ) : null} - {children} - - - ) -} - -export default ChannelSkeletonProps diff --git a/frontends/mit-open/src/pages/ChannelPage/ChannelPageTemplate.tsx b/frontends/mit-open/src/pages/ChannelPage/ChannelPageTemplate.tsx new file mode 100644 index 0000000000..5d9a10eedb --- /dev/null +++ b/frontends/mit-open/src/pages/ChannelPage/ChannelPageTemplate.tsx @@ -0,0 +1,95 @@ +import React from "react" +import UnitChannelSkeleton from "./UnitChannelTemplate" +import DefaultChannelSkeleton from "./DefaultChannelTemplate" +import { ChannelTypeEnum } from "api/v0" +import { + DEPARTMENTS as DEPARTMENTS_URL, + TOPICS as TOPICS_URL, + UNITS as UNITS_URL, +} from "@/common/urls" +import { styled } from "ol-components" + +const TOPICS_LABEL = "Browse by Topic" +const DEPARTMENTS_LABEL = "Browse by Academic Department" +const UNITS_LABEL = "MIT Units" +const PATHWAYS_LABEL = "Pathways" + +const CHANNEL_TYPE_BREADCRUMB_TARGETS: { + [key: string]: { href: string; label: string } +} = { + topic: { + href: TOPICS_URL, + label: TOPICS_LABEL, + }, + department: { + href: DEPARTMENTS_URL, + label: DEPARTMENTS_LABEL, + }, + unit: { + href: UNITS_URL, + label: UNITS_LABEL, + }, + pathway: { + href: "", + label: PATHWAYS_LABEL, + }, +} + +const ChannelTitleRow = styled.div({ + display: "flex", + flexDirection: "row", + alignItems: "center", + + h1: { + a: { + "&:hover": { + textDecoration: "none", + }, + }, + }, +}) + +const ChannelControls = styled.div({ + position: "relative", + minHeight: "38px", + display: "flex", +}) + +interface ChannelSkeletonProps { + children: React.ReactNode + channelType: string + name: string +} + +/** + * Common structure for channel-oriented pages. + * + * Renders the channel title and avatar in a banner. + */ +const ChannelPageTemplate: React.FC = ({ + children, + channelType, + name, +}) => { + const ChannelTemplate = + channelType === ChannelTypeEnum.Unit + ? UnitChannelSkeleton + : DefaultChannelSkeleton + + return ( + + {children} + + ) +} + +export { + ChannelPageTemplate, + ChannelTitleRow, + ChannelControls, + CHANNEL_TYPE_BREADCRUMB_TARGETS, + UNITS_LABEL, + TOPICS_LABEL, + DEPARTMENTS_LABEL, + PATHWAYS_LABEL, +} diff --git a/frontends/mit-open/src/pages/ChannelPage/ChannelSearch.test.tsx b/frontends/mit-open/src/pages/ChannelPage/ChannelSearch.test.tsx index 2556c72227..80c8fc2f00 100644 --- a/frontends/mit-open/src/pages/ChannelPage/ChannelSearch.test.tsx +++ b/frontends/mit-open/src/pages/ChannelPage/ChannelSearch.test.tsx @@ -116,7 +116,7 @@ describe("ChannelSearch", () => { }) setMockResponse.get(urls.userMe.get(), {}) renderTestApp({ url: `/c/${channel.channel_type}/${channel.name}` }) - await screen.findByText(channel.title) + await screen.findAllByText(channel.title) const tabpanel = await screen.findByRole("tabpanel") for (const resource of resources) { await within(tabpanel).findByText(resource.title) diff --git a/frontends/mit-open/src/pages/ChannelPage/DefaultChannelTemplate.tsx b/frontends/mit-open/src/pages/ChannelPage/DefaultChannelTemplate.tsx new file mode 100644 index 0000000000..a5614219dc --- /dev/null +++ b/frontends/mit-open/src/pages/ChannelPage/DefaultChannelTemplate.tsx @@ -0,0 +1,165 @@ +import React from "react" +import { + styled, + Container, + Typography, + Breadcrumbs, + Banner, +} from "ol-components" +import { MetaTags } from "ol-utilities" +import { SearchSubscriptionToggle } from "@/page-components/SearchSubscriptionToggle/SearchSubscriptionToggle" +import { useChannelDetail } from "api/hooks/channels" +import ChannelMenu from "@/components/ChannelMenu/ChannelMenu" +import ChannelAvatar from "@/components/ChannelAvatar/ChannelAvatar" +import { SourceTypeEnum } from "api" +import { HOME as HOME_URL } from "../../common/urls" +import { + CHANNEL_TYPE_BREADCRUMB_TARGETS, + ChannelControls, + ChannelTitleRow, +} from "./ChannelPageTemplate" + +const HeadingTextContainer = styled.div(({ theme }) => ({ + display: "flex", + flexDirection: "row", + alignItems: "center", + flexGrow: 0, + flexShrink: 0, + order: 2, + my: 1, + [theme.breakpoints.down("sm")]: { + width: "100%", + }, + [theme.breakpoints.up("md")]: { + width: "80%", + }, +})) + +const ChannelControlsContainer = styled.div(({ theme }) => ({ + display: "flex", + flexDirection: "row", + alignItems: "end", + flexGrow: 0, + flexShrink: 0, + order: 2, + [theme.breakpoints.down("xs")]: { + width: "100%", + }, + [theme.breakpoints.down("sm")]: { + mt: "8px", + mb: "48px", + }, + [theme.breakpoints.up("md")]: { + mt: "0px", + mb: "48px", + width: "15%", + }, +})) + +interface DefaultChannelTemplateProps { + children: React.ReactNode + channelType: string + name: string +} + +/** + * Common structure for channel-oriented pages. + * + * Renders the channel title and avatar in a banner. + */ +const DefaultChannelTemplate: React.FC = ({ + children, + channelType, + name, +}) => { + const channel = useChannelDetail(String(channelType), String(name)) + const urlParams = new URLSearchParams(channel.data?.search_filter) + const displayConfiguration = channel.data?.configuration + + return ( + <> + + + } + avatar={ + displayConfiguration?.logo && + channel.data && ( + + ) + } + header={channel.data?.title} + subheader={displayConfiguration?.heading} + extraHeader={displayConfiguration?.sub_heading} + backgroundUrl={ + displayConfiguration?.banner_background ?? + "/static/images/background_steps.jpeg" + } + extraRight={ + + + {channel.data?.search_filter ? ( + + ) : null} + {channel.data?.is_moderator ? ( + + ) : null} + + + } + /> + + + {displayConfiguration?.heading ? ( + + + {displayConfiguration.heading} + + + ) : ( + <> + )} + {displayConfiguration?.sub_heading ? ( + + + {displayConfiguration.sub_heading} + + + ) : ( + <> + )} + + + {children} + + ) +} + +export default DefaultChannelTemplate diff --git a/frontends/mit-open/src/pages/ChannelPage/EditChannelPage.tsx b/frontends/mit-open/src/pages/ChannelPage/EditChannelPage.tsx index d5581ee8bc..82eae36dfd 100644 --- a/frontends/mit-open/src/pages/ChannelPage/EditChannelPage.tsx +++ b/frontends/mit-open/src/pages/ChannelPage/EditChannelPage.tsx @@ -8,7 +8,7 @@ import { MetaTags } from "ol-utilities" import { GridColumn, GridContainer } from "@/components/GridLayout/GridLayout" import { useChannelDetail } from "api/hooks/channels" import EditChannelAppearanceForm from "./EditChannelAppearanceForm" -import ChannelPageSkeleton from "./ChannelPageSkeleton" +import { ChannelPageTemplate } from "./ChannelPageTemplate" type RouteParams = { channelType: string name: string @@ -34,7 +34,7 @@ const EditChannelPage: React.FC = () => { ) return channel.data ? ( - @@ -89,7 +89,7 @@ const EditChannelPage: React.FC = () => { )} - + ) : null } diff --git a/frontends/mit-open/src/pages/ChannelPage/UnitChannelTemplate.tsx b/frontends/mit-open/src/pages/ChannelPage/UnitChannelTemplate.tsx new file mode 100644 index 0000000000..7e200a0dd9 --- /dev/null +++ b/frontends/mit-open/src/pages/ChannelPage/UnitChannelTemplate.tsx @@ -0,0 +1,184 @@ +import React, { useMemo } from "react" +import { styled, Container, Box, Breadcrumbs, Banner } from "ol-components" +import { MetaTags } from "ol-utilities" +import { SearchSubscriptionToggle } from "@/page-components/SearchSubscriptionToggle/SearchSubscriptionToggle" +import { ChannelDetails } from "@/page-components/ChannelDetails/ChannelDetails" +import { useChannelDetail } from "api/hooks/channels" +import ChannelMenu from "@/components/ChannelMenu/ChannelMenu" +import ChannelAvatar from "@/components/ChannelAvatar/ChannelAvatar" +import ResourceCarousel, { + ResourceCarouselProps, +} from "@/page-components/ResourceCarousel/ResourceCarousel" +import { SourceTypeEnum } from "api" +import { getSearchParamMap } from "@/common/utils" +import { HOME as HOME_URL, UNITS as UNITS_URL } from "../../common/urls" +import { ChannelTypeEnum } from "api/v0" +import { ChannelControls, UNITS_LABEL } from "./ChannelPageTemplate" + +const FeaturedCoursesCarousel = styled(ResourceCarousel)(({ theme }) => ({ + margin: "80px 0", + [theme.breakpoints.down("sm")]: { + marginTop: "32px", + marginBottom: "32px", + }, +})) + +interface UnitChannelTemplateProps { + children: React.ReactNode + name: string +} + +/** + * Common structure for channel-oriented pages. + * + * Renders the channel title and avatar in a banner. + */ +const UnitChannelTemplate: React.FC = ({ + children, + name, +}) => { + const channel = useChannelDetail(ChannelTypeEnum.Unit, String(name)) + const urlParams = new URLSearchParams(channel.data?.search_filter) + const displayConfiguration = channel.data?.configuration + + const urlParamMap: Record = useMemo(() => { + const urlParams = new URLSearchParams(channel.data?.search_filter) + return getSearchParamMap(urlParams) + }, [channel]) + + const FEATURED_RESOURCES_CAROUSEL: ResourceCarouselProps["config"] = [ + { + cardProps: { size: "medium" }, + data: { + type: "lr_featured", + params: { limit: 12, ...urlParamMap }, + }, + label: undefined, + }, + ] + + const headerStyles = { + width: { md: "80%", sm: "100%" }, + my: 1, + } + + return ( + <> + + + } + avatar={ + displayConfiguration?.logo && channel.data ? ( + ({ + flexGrow: 1, + flexShrink: 0, + order: 1, + py: "24px", + + [theme.breakpoints.down("md")]: { + py: 0, + pb: "8px", + }, + [theme.breakpoints.down("sm")]: { + pt: "32px", + width: "100%", + }, + })} + > + + + ) : null + } + header={displayConfiguration?.heading} + headerTypography={{ xs: "h5", md: "h4" }} + headerStyles={headerStyles} + subheader={displayConfiguration?.sub_heading} + subheaderStyles={headerStyles} + subheaderTypography={{ xs: "body2", md: "body1" }} + extraHeader={ + + + {channel.data?.search_filter ? ( + + ) : null} + {channel.data?.is_moderator ? ( + + ) : null} + + + } + extraRight={ + channel.data && ( + + + + ) + } + /> + + + {children} + + + ) +} + +export default UnitChannelTemplate diff --git a/frontends/mit-open/src/pages/DepartmentListingPage/DepartmentListingPage.tsx b/frontends/mit-open/src/pages/DepartmentListingPage/DepartmentListingPage.tsx index f40a37f645..cbdbd7d653 100644 --- a/frontends/mit-open/src/pages/DepartmentListingPage/DepartmentListingPage.tsx +++ b/frontends/mit-open/src/pages/DepartmentListingPage/DepartmentListingPage.tsx @@ -217,8 +217,8 @@ const DepartmentListingPage: React.FC = () => { { current="Topics" /> } - title="Browse by Topic" - description="" + header="Browse by Topic" + subheader="" backgroundUrl={TOPICS_BANNER_IMAGE} /> diff --git a/frontends/mit-open/src/pages/UnitsListingPage/UnitsListingPage.tsx b/frontends/mit-open/src/pages/UnitsListingPage/UnitsListingPage.tsx index b6b0e556ed..ff376a8258 100644 --- a/frontends/mit-open/src/pages/UnitsListingPage/UnitsListingPage.tsx +++ b/frontends/mit-open/src/pages/UnitsListingPage/UnitsListingPage.tsx @@ -414,8 +414,8 @@ const UnitsListingPage: React.FC = () => { current="MIT Units" /> } - title="Academic & Professional Learning" - description="Non-degree learning resources tailored to the needs of students and working professionals." + header="Academic & Professional Learning" + subheader="Non-degree learning resources tailored to the needs of students and working professionals." backgroundUrl={UNITS_BANNER_IMAGE} /> diff --git a/frontends/ol-components/src/components/Banner/Banner.stories.tsx b/frontends/ol-components/src/components/Banner/Banner.stories.tsx index f145eae1be..59e2922190 100644 --- a/frontends/ol-components/src/components/Banner/Banner.stories.tsx +++ b/frontends/ol-components/src/components/Banner/Banner.stories.tsx @@ -1,21 +1,94 @@ +import React from "react" import type { Meta, StoryObj } from "@storybook/react" +import { withRouter } from "storybook-addon-react-router-v6" import { Banner } from "./Banner" +import { Breadcrumbs } from "../Breadcrumbs/Breadcrumbs" +import { Button } from "../Button/Button" +import Typography from "@mui/material/Typography" const lipsum = "Lorem ipsum dolor sit amet, consectetur adipiscing elit. Sed nonne merninisti licere mihi ista probare, quae sunt a te dicta? Refert tamen, quo modo" -const meta: Meta = { +const meta: Meta = { title: "smoot-design/Banner", component: Banner, args: { backgroundUrl: "https://images.pexels.com/photos/1851188/pexels-photo-1851188.jpeg?auto=compress&cs=tinysrgb&w=800", - title: "Banner Title", - description: lipsum, - navText: "Home / Nav / Text", + navText: ( + + ), + header: "Banner Title", + subheader: lipsum, }, } export default meta type Story = StoryObj -export const story: Story = {} + +export const basicBanner: Story = { + decorators: [withRouter], + render: (args) => , +} + +export const logoBanner: Story = { + decorators: [withRouter], + render: (args) => { + return ( + + } + {...args} + /> + ) + }, +} + +export const logoBannerWithExtras: Story = { + decorators: [withRouter], + render: (args) => { + return ( + + } + extraHeader={ + + } + extraRight={ +
+ Extra Content +
+ +
+
+ +
+
+ } + {...args} + /> + ) + }, +} diff --git a/frontends/ol-components/src/components/Banner/Banner.tsx b/frontends/ol-components/src/components/Banner/Banner.tsx index 29a0556378..06668ab6ce 100644 --- a/frontends/ol-components/src/components/Banner/Banner.tsx +++ b/frontends/ol-components/src/components/Banner/Banner.tsx @@ -2,45 +2,84 @@ import React from "react" import styled from "@emotion/styled" import Typography from "@mui/material/Typography" import Container from "@mui/material/Container" +import { ResponsiveStyleValue, SxProps } from "@mui/system" +import { Theme } from "../ThemeProvider/ThemeProvider" -const Description = styled(Typography)(({ theme }) => ({ +const SubHeader = styled(Typography)({ maxWidth: "700px", marginTop: "8px", - [theme.breakpoints.down("sm")]: { - marginTop: "16px", - }, -})) +}) type BannerWrapperProps = { backgroundUrl: string + backgroundSize?: string + backgroundDim?: number + containerPadding?: string } /** * This is a full-width banner component that takes a background image URL. */ -const BannerWrapper = styled.div( - ({ theme, backgroundUrl }) => ({ - backgroundImage: `url(${backgroundUrl})`, - backgroundSize: "cover", +const BannerWrapper = styled.header( + ({ + theme, + backgroundUrl, + backgroundSize = "cover", + backgroundDim = 0, + containerPadding = "48px 0 48px 0", + }) => ({ + backgroundAttachment: "fixed", + backgroundImage: backgroundDim + ? `linear-gradient(rgba(0 0 0 / ${backgroundDim}%), rgba(0 0 0 / ${backgroundDim}%)), url('${backgroundUrl}')` + : `url(${backgroundUrl})`, + backgroundSize: backgroundSize, + backgroundRepeat: "no-repeat", color: theme.custom.colors.white, - paddingTop: "48px", - paddingBottom: "48px", + padding: containerPadding, [theme.breakpoints.down("sm")]: { - paddingTop: "32px", - paddingBottom: "32px", + padding: "32px 0 32px 0", }, }), ) -const NavText = styled(Typography)(({ theme }) => ({ - color: theme.custom.colors.lightGray2, - marginBottom: "4px", +const InnerContainer = styled.div(({ theme }) => ({ + display: "flex", + flexDirection: "row", + alignItems: "flex-start", + justifyContent: "space-between", + [theme.breakpoints.down("md")]: { + flexDirection: "column", + }, +})) + +const HeaderContainer = styled.div({ + display: "flex", + flexDirection: "column", +}) + +const RightContainer = styled.div(({ theme }) => ({ + display: "flex", + flexDirection: "row", + [theme.breakpoints.down("md")]: { + width: "100%", + }, })) type BannerProps = BannerWrapperProps & { - description: React.ReactNode - title: React.ReactNode + backgroundUrl: string + backgroundSize?: string + backgroundDim?: number + containerPadding?: string navText: React.ReactNode + avatar?: React.ReactNode + header: React.ReactNode + headerTypography?: ResponsiveStyleValue + headerStyles?: SxProps + subheader?: React.ReactNode + subheaderTypography?: ResponsiveStyleValue + subheaderStyles?: SxProps + extraHeader?: React.ReactNode + extraRight?: React.ReactNode } /** @@ -49,20 +88,52 @@ type BannerProps = BannerWrapperProps & { */ const Banner = ({ backgroundUrl, - description, - title, + backgroundSize = "cover", + backgroundDim = 0, + containerPadding = "48px 0 48px 0", navText, + avatar, + header, + headerTypography, + headerStyles, + subheader, + subheaderTypography, + subheaderStyles, + extraHeader, + extraRight, }: BannerProps) => { + const defaultHeaderTypography = { xs: "h2", md: "h1" } + const defaultSubHeaderTypography = { xs: "body2", md: "body1" } return ( - + - {navText} - - {title} - - - {description} - + {navText} + + + {avatar ?
{avatar}
: null} + + {header} + + + {subheader} + +
{extraHeader}
+ + {extraRight} + ) diff --git a/frontends/ol-components/src/components/Breadcrumbs/Breadcrumbs.test.tsx b/frontends/ol-components/src/components/Breadcrumbs/Breadcrumbs.test.tsx index f2cbfcc1f6..44b8c28159 100644 --- a/frontends/ol-components/src/components/Breadcrumbs/Breadcrumbs.test.tsx +++ b/frontends/ol-components/src/components/Breadcrumbs/Breadcrumbs.test.tsx @@ -46,7 +46,7 @@ describe("Breadcrumbs", () => { ) expect(screen.getAllByRole("link")).toHaveLength(totalAncestors) expectedLabels.forEach((label, index) => { - const link = screen.getByText(label) + const link = screen.getByText(label).parentElement expect(link).toBeInTheDocument() expect(link).toHaveAttribute("href", expectedHrefs[index]) }) diff --git a/frontends/ol-components/src/components/Breadcrumbs/Breadcrumbs.tsx b/frontends/ol-components/src/components/Breadcrumbs/Breadcrumbs.tsx index 8389b5fb61..60c7f1ec7b 100644 --- a/frontends/ol-components/src/components/Breadcrumbs/Breadcrumbs.tsx +++ b/frontends/ol-components/src/components/Breadcrumbs/Breadcrumbs.tsx @@ -8,11 +8,26 @@ const BreadcrumbsContainer = styled.span({ display: "inline-flex", paddingBottom: "16px", alignItems: "flex-start", + overflow: "hidden", + width: "100%", }) const Breadcrumb = styled.span({ display: "flex", alignItems: "center", + overflow: "hidden", +}) + +const BreadcrumbLink = styled(Link)({ + overflow: "hidden", + textOverflow: "ellipsis", +}) + +const BreadcrumbText = styled.span({ + textWrap: "nowrap", + whiteSpace: "nowrap", + overflow: "hidden", + textOverflow: "ellipsis", }) const Separator = styled(RiArrowRightSLine)({ @@ -28,7 +43,7 @@ const DarkSeparator = styled(Separator)({ color: theme.custom.colors.silverGray, }) -const Current = styled.span({ +const Current = styled(BreadcrumbText)({ ...theme.typography.body3, }) @@ -57,14 +72,14 @@ const Breadcrumbs: React.FC = (props) => { const isLast = index === ancestors.length return ( - - {ancestor.label} - + {ancestor.label} + {!isLast && <_Separator data-testid="breadcrumb-separator" />} ) From d9cbccc5e16bad8f26c6fe05ac4d9bdf96a7ac9e Mon Sep 17 00:00:00 2001 From: Matt Bertrand Date: Thu, 11 Jul 2024 12:45:53 -0400 Subject: [PATCH 05/11] Change readable_id values for podcasts and episodes (#1232) --- learning_resources/etl/loaders.py | 23 +++++-- learning_resources/etl/podcast.py | 45 +++++++------ learning_resources/etl/podcast_test.py | 27 +++----- .../commands/transfer_list_resources.py | 63 +++++++++++++++++++ ...0056_alter_learningresource_readable_id.py | 17 +++++ learning_resources/models.py | 2 +- learning_resources/utils.py | 53 ++++++++++++++++ learning_resources/utils_test.py | 61 ++++++++++++++++++ openapi/specs/v1.yaml | 24 +++---- 9 files changed, 261 insertions(+), 54 deletions(-) create mode 100644 learning_resources/management/commands/transfer_list_resources.py create mode 100644 learning_resources/migrations/0056_alter_learningresource_readable_id.py diff --git a/learning_resources/etl/loaders.py b/learning_resources/etl/loaders.py index b2076c60a5..91020fedf1 100644 --- a/learning_resources/etl/loaders.py +++ b/learning_resources/etl/loaders.py @@ -682,6 +682,10 @@ def load_podcast(podcast_data: dict) -> LearningResource: LearningResource.objects.filter(id__in=unpublished_episode_ids).update( published=False ) + bulk_resources_unpublished_actions( + unpublished_episode_ids, + LearningResourceType.podcast_episode.name, + ) episode_ids.extend(unpublished_episode_ids) learning_resource.resources.set( episode_ids, @@ -719,13 +723,22 @@ def load_podcasts(podcasts_data: list[dict]) -> list[LearningResource]: # unpublish the podcasts and episodes we're no longer tracking ids = [podcast.id for podcast in podcast_resources] - LearningResource.objects.filter( + unpublished_podcasts = LearningResource.objects.filter( resource_type=LearningResourceType.podcast.name - ).exclude(id__in=ids).update(published=False) - LearningResource.objects.filter( + ).exclude(id__in=ids) + unpublished_podcasts.update(published=False) + bulk_resources_unpublished_actions( + unpublished_podcasts.values_list("id", flat=True), + LearningResourceType.podcast.name, + ) + unpublished_episodes = LearningResource.objects.filter( resource_type=LearningResourceType.podcast_episode.name - ).exclude(parents__parent__in=ids).update(published=False) - + ).exclude(parents__parent__in=ids) + unpublished_episodes.update(published=False) + bulk_resources_unpublished_actions( + unpublished_episodes.values_list("id", flat=True), + LearningResourceType.podcast_episode.name, + ) return podcast_resources diff --git a/learning_resources/etl/podcast.py b/learning_resources/etl/podcast.py index 332dc944b0..a6e40379a8 100644 --- a/learning_resources/etl/podcast.py +++ b/learning_resources/etl/podcast.py @@ -12,7 +12,7 @@ from learning_resources.constants import LearningResourceType from learning_resources.etl.constants import ETLSource -from learning_resources.etl.utils import clean_data, generate_readable_id +from learning_resources.etl.utils import clean_data from learning_resources.models import PodcastEpisode from main.utils import frontend_absolute_url, now_in_utc @@ -100,6 +100,19 @@ def get_podcast_configs(): return podcast_configs +def parse_readable_id_from_url(url): + """ + Parse readable id from podcast/episode url + + Args: + url (str): the podcast/episode url + + Returns: + str: the readable id + """ + return url.split("//")[-1] + + def extract(): """ Function for extracting podcast data @@ -125,7 +138,7 @@ def extract(): log.exception("Invalid rss url %s", rss_url) -def transform_episode(rss_data, offered_by, topics, parent_image, podcast_id): +def transform_episode(rss_data, offered_by, topics, parent_image): """ Transform a podcast episode into our normalized data @@ -140,10 +153,11 @@ def transform_episode(rss_data, offered_by, topics, parent_image, podcast_id): normalized podcast episode data """ - rss_data.guid.string = f"{podcast_id}: {rss_data.guid.text}" - return { - "readable_id": generate_readable_id(rss_data.title.text[:95]), + "readable_id": rss_data.guid.text + or parse_readable_id_from_url( + rss_data.link.text if rss_data.link else rss_data.enclosure["url"] + ), "etl_source": ETLSource.podcast.name, "resource_type": LearningResourceType.podcast_episode.name, "title": rss_data.title.text, @@ -151,12 +165,10 @@ def transform_episode(rss_data, offered_by, topics, parent_image, podcast_id): "description": clean_data(rss_data.description.text), "url": rss_data.enclosure["url"], "image": { - "url": ( - rss_data.find("image")["href"] - if rss_data.find("image") - else parent_image - ), - }, + "url": (rss_data.find("image")["href"]), + } + if rss_data.find("image") + else parent_image, "last_modified": parse(rss_data.pubDate.text), "published": True, "topics": topics, @@ -186,7 +198,7 @@ def transform(extracted_podcasts): for rss_data, config_data in extracted_podcasts: try: image = ( - rss_data.channel.find("itunes:image")["href"] + {"url": rss_data.channel.find("itunes:image")["href"]} if rss_data.channel.find("itunes:image") else None ) @@ -203,23 +215,20 @@ def transform(extracted_podcasts): apple_podcasts_url = config_data.get("apple_podcasts_url") google_podcasts_url = config_data.get("google_podcasts_url") title = config_data.get("podcast_title", rss_data.channel.title.text) - podcast_id = generate_readable_id(title[:95]) yield { - "readable_id": podcast_id, + "readable_id": parse_readable_id_from_url(config_data["rss_url"]), "title": title, "etl_source": ETLSource.podcast.name, "resource_type": LearningResourceType.podcast.name, "offered_by": offered_by, "description": clean_data(rss_data.channel.description.text), - "image": {"url": rss_data.channel.find("itunes:image")["href"]}, + "image": image, "published": True, "url": config_data["website"], "topics": topics, "episodes": ( - transform_episode( - episode_rss, offered_by, topics, image, podcast_id - ) + transform_episode(episode_rss, offered_by, topics, image) for episode_rss in rss_data.find_all("item") ), "podcast": { diff --git a/learning_resources/etl/podcast_test.py b/learning_resources/etl/podcast_test.py index 3012c6de62..f48ec454c8 100644 --- a/learning_resources/etl/podcast_test.py +++ b/learning_resources/etl/podcast_test.py @@ -43,23 +43,22 @@ def rss_content(): def mock_podcast_file( # pylint: disable=too-many-arguments # noqa: PLR0913 podcast_title=None, topics=None, - website_url="website_url", + website_url="http://website.url/podcast", offered_by=None, google_podcasts_url="google_podcasts_url", apple_podcasts_url="apple_podcasts_url", - rss_url="rss_url", + rss_url="http://website.url/podcast/rss.xml", ): """Mock podcast github file""" content = f"""--- -rss_url: rss_url +rss_url: {rss_url} { "podcast_title: " + podcast_title if podcast_title else "" } { "topics: " + topics if topics else "" } { "offered_by: " + offered_by if offered_by else "" } website: {website_url} google_podcasts_url: {google_podcasts_url} apple_podcasts_url: {apple_podcasts_url} -rss_url: {rss_url} """ return Mock(decoded_content=content) @@ -123,22 +122,14 @@ def test_transform(mock_github_client, title, topics, offered_by): ) expected_title = title if title else "A Podcast" - expected_readable_id = ( - "custom-titleb04b26d38dd63a2c829393e9e075927d" - if title - else "a-podcast7e3a1ebb0c4d3196ba4c7f8254af4d2d" - ) expected_offered_by = {"name": offered_by} if offered_by else None episodes_rss = list(bs(rss_content(), "xml").find_all("item")) - for episode in episodes_rss: - episode.guid.string = f"{expected_readable_id}: {episode.guid.text}" - expected_results = [ { - "readable_id": expected_readable_id, + "readable_id": "website.url/podcast/rss.xml", "etl_source": ETLSource.podcast.name, "title": expected_title, "offered_by": expected_offered_by, @@ -149,13 +140,13 @@ def test_transform(mock_github_client, title, topics, offered_by): "podcast": { "google_podcasts_url": "google_podcasts_url", "apple_podcasts_url": "apple_podcasts_url", - "rss_url": "rss_url", + "rss_url": "http://website.url/podcast/rss.xml", }, "resource_type": LearningResourceType.podcast.name, "topics": expected_topics, "episodes": [ { - "readable_id": "episode15ede89915db9342fb76bc91918d22016", + "readable_id": "tag:soundcloud,2010:tracks/numbers1", "etl_source": ETLSource.podcast.name, "title": "Episode1", "offered_by": expected_offered_by, @@ -178,7 +169,7 @@ def test_transform(mock_github_client, title, topics, offered_by): "topics": expected_topics, }, { - "readable_id": "episode205c066df9ed531e48c6414f6e72d3b96", + "readable_id": "tag:soundcloud,2010:tracks/numbers2", "etl_source": ETLSource.podcast.name, "title": "Episode2", "offered_by": expected_offered_by, @@ -229,11 +220,11 @@ def test_transform_with_error(mocker, mock_github_client): results = list(transform(extract_results)) mock_exception_log.assert_called_once_with( - "Error parsing podcast data from %s", "rss_url" + "Error parsing podcast data from %s", "http://website.url/podcast/rss.xml" ) assert len(results) == 1 - assert results[0]["url"] == "website_url" + assert results[0]["url"] == "http://website.url/podcast" @pytest.mark.django_db() diff --git a/learning_resources/management/commands/transfer_list_resources.py b/learning_resources/management/commands/transfer_list_resources.py new file mode 100644 index 0000000000..03b4ab4e36 --- /dev/null +++ b/learning_resources/management/commands/transfer_list_resources.py @@ -0,0 +1,63 @@ +"""Management command for migrating unpublished resource path/list relationships""" + +from django.core.management import BaseCommand + +from learning_resources.utils import transfer_list_resources +from main.utils import now_in_utc + + +class Command(BaseCommand): + """ + Migrate relationships in learningpaths and userlists from unpublished + resources to published replacement resources. + """ + + help = "Migrate relationships from unpublished resources to published resources." + + def add_arguments(self, parser): + parser.add_argument("resource_type", type=str, help="Resource type to migrate") + parser.add_argument( + "match_field", type=str, help="Resource field to match resources by" + ) + parser.add_argument( + "from_source", type=str, help="ETL Source for unpublished resources" + ) + parser.add_argument( + "to_source", type=str, help="ETL Source for published resources" + ) + parser.add_argument( + "--delete", + dest="delete", + action="store_true", + help="Delete unpublished resources after migrating relationships", + ) + super().add_arguments(parser) + + def handle(self, *args, **options): # noqa: ARG002 + """ + Migrate relationships in learningpaths and userlists from unpublished + resources to published replacement resources. + """ + resource_type = options["resource_type"] + match_field = options["match_field"] + from_source = options["from_source"] + to_source = options["to_source"] + delete = options["delete"] + + self.stdout.write( + f"Migrate {resource_type} relationships from " + f"{from_source} to {to_source}, matching on {match_field}" + ) + start = now_in_utc() + unpublished, matching = transfer_list_resources( + resource_type, + match_field, + from_source, + to_source, + delete_unpublished=delete, + ) + total_seconds = (now_in_utc() - start).total_seconds() + self.stdout.write( + f"Processed {unpublished} resources and found {matching} " + f"published matches, took {total_seconds} seconds" + ) diff --git a/learning_resources/migrations/0056_alter_learningresource_readable_id.py b/learning_resources/migrations/0056_alter_learningresource_readable_id.py new file mode 100644 index 0000000000..69ee099aeb --- /dev/null +++ b/learning_resources/migrations/0056_alter_learningresource_readable_id.py @@ -0,0 +1,17 @@ +# Generated by Django 4.2.13 on 2024-07-08 16:30 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + dependencies = [ + ("learning_resources", "0055_alter_relationship_options_ordering"), + ] + + operations = [ + migrations.AlterField( + model_name="learningresource", + name="readable_id", + field=models.CharField(max_length=512), + ), + ] diff --git a/learning_resources/models.py b/learning_resources/models.py index 5e1fa5a9ad..4de9909f74 100644 --- a/learning_resources/models.py +++ b/learning_resources/models.py @@ -185,7 +185,7 @@ class LearningResource(TimestampedModel): *([item.name for item in LearningResourceType]), ] - readable_id = models.CharField(max_length=128, null=False, blank=False) + readable_id = models.CharField(max_length=512, null=False, blank=False) title = models.CharField(max_length=256) description = models.TextField(null=True, blank=True) # noqa: DJ001 full_description = models.TextField(null=True, blank=True) # noqa: DJ001 diff --git a/learning_resources/utils.py b/learning_resources/utils.py index e22d43230d..90b3795cab 100644 --- a/learning_resources/utils.py +++ b/learning_resources/utils.py @@ -16,6 +16,7 @@ from learning_resources.constants import ( GROUP_STAFF_LISTS_EDITORS, + LearningResourceRelationTypes, semester_mapping, ) from learning_resources.hooks import get_plugin_manager @@ -24,9 +25,11 @@ LearningResourceDepartment, LearningResourceOfferor, LearningResourcePlatform, + LearningResourceRelationship, LearningResourceRun, LearningResourceSchool, LearningResourceTopic, + UserListRelationship, ) from main.utils import generate_filepath @@ -559,3 +562,53 @@ def add_parent_topics_to_learning_resource(resource): for topic in resource.topics.all(): if topic.parent: _walk_lr_topic_parents(resource, topic.parent) + + +def transfer_list_resources( + resource_type: str, + matching_field: str, + from_source: str, + to_source: str, + *, + delete_unpublished: bool = False, +) -> tuple[int, int]: + """ + Migrate unpublished learningpath/userlist resources that have + been replaced with new resource objects. + + Args: + resource_type (str): the resource type + matching_field (str): the unique field to match identical resources + from_source (str): the ETL source of unpublished resources + to_source (str): the ETL source of published resources + delete_unpublished (bool): whether to delete the unpublished resources + + Returns: + tuple[int, int]: the number of unpublished and matching published resources + """ + unpublished_resources = LearningResource.objects.filter( + resource_type=resource_type, published=False, etl_source=from_source + ) + unpublished_count = 0 + published_count = 0 + for resource in unpublished_resources: + unpublished_count += 1 + unique_value = getattr(resource, matching_field) + published_replacement = LearningResource.objects.filter( + **{matching_field: unique_value}, + resource_type=resource_type, + published=True, + etl_source=to_source, + ).first() + if published_replacement is not None: + published_count += 1 + LearningResourceRelationship.objects.filter( + relation_type=LearningResourceRelationTypes.LEARNING_PATH_ITEMS.value, + child=resource, + ).update(child=published_replacement) + UserListRelationship.objects.filter(child=resource).update( + child=published_replacement + ) + if delete_unpublished: + unpublished_resources.delete() + return unpublished_count, published_count diff --git a/learning_resources/utils_test.py b/learning_resources/utils_test.py index 8e83946a5e..177d7a0c5f 100644 --- a/learning_resources/utils_test.py +++ b/learning_resources/utils_test.py @@ -12,20 +12,26 @@ CONTENT_TYPE_FILE, CONTENT_TYPE_PDF, CONTENT_TYPE_VIDEO, + LearningResourceRelationTypes, ) from learning_resources.etl.utils import get_content_type from learning_resources.factories import ( CourseFactory, + LearningPathFactory, + LearningResourceFactory, LearningResourceRunFactory, LearningResourceTopicFactory, + UserListFactory, ) from learning_resources.models import ( + LearningResource, LearningResourceOfferor, LearningResourcePlatform, LearningResourceTopic, ) from learning_resources.utils import ( add_parent_topics_to_learning_resource, + transfer_list_resources, upsert_topic_data, ) @@ -324,3 +330,58 @@ def test_upsert_offered_by(mocker): ) assert offeror.name == offered_by_data["fields"]["name"] mock_upsert.assert_any_call(offeror, overwrite=True) + + +@pytest.mark.parametrize( + ("matching_field", "from_source", "to_source", "matches", "delete_old"), + [ + ("url", "podcast", "podcast", True, False), + ("url", "podcast", "podcast", True, True), + ("url", "podcast", "xpro", False, False), + ("readable_id", "podcast", "podcast", False, False), + ], +) +def test_transfer_list_resources( + matching_field, from_source, to_source, matches, delete_old +): + """Test that the transfer_list_resources function works as expected.""" + original_podcasts = LearningResourceFactory.create_batch( + 5, is_podcast=True, etl_source=from_source, published=False + ) + podcast_path = LearningPathFactory.create().learning_resource + podcast_path.resources.set( + original_podcasts, + through_defaults={ + "relation_type": LearningResourceRelationTypes.LEARNING_PATH_ITEMS + }, + ) + podcast_list = UserListFactory.create() + podcast_list.resources.set(original_podcasts) + + new_podcasts = [ + LearningResourceFactory.create( + is_podcast=True, url=old_podcast.url, etl_source=from_source + ) + for old_podcast in original_podcasts + ] + + results = transfer_list_resources( + "podcast", matching_field, from_source, to_source, delete_unpublished=delete_old + ) + podcast_path.refresh_from_db() + podcast_list.refresh_from_db() + + assert results == ((5, 5) if matches else (5, 0)) + list_podcasts = ( + new_podcasts if matches else (original_podcasts if not delete_old else []) + ) + for podcast in list_podcasts: + assert podcast in podcast_path.resources.all() + assert podcast in podcast_list.resources.all() + if delete_old: + assert ( + LearningResource.objects.filter( + id__in=[podcast.id for podcast in original_podcasts] + ).count() + == 0 + ) diff --git a/openapi/specs/v1.yaml b/openapi/specs/v1.yaml index 1517ae72fc..630d0ed113 100644 --- a/openapi/specs/v1.yaml +++ b/openapi/specs/v1.yaml @@ -7531,7 +7531,7 @@ components: readOnly: true readable_id: type: string - maxLength: 128 + maxLength: 512 title: type: string maxLength: 256 @@ -7598,7 +7598,7 @@ components: readable_id: type: string minLength: 1 - maxLength: 128 + maxLength: 512 title: type: string minLength: 1 @@ -9815,7 +9815,7 @@ components: readOnly: true readable_id: type: string - maxLength: 128 + maxLength: 512 title: type: string maxLength: 256 @@ -9882,7 +9882,7 @@ components: readable_id: type: string minLength: 1 - maxLength: 128 + maxLength: 512 title: type: string minLength: 1 @@ -10065,7 +10065,7 @@ components: readOnly: true readable_id: type: string - maxLength: 128 + maxLength: 512 title: type: string maxLength: 256 @@ -10132,7 +10132,7 @@ components: readable_id: type: string minLength: 1 - maxLength: 128 + maxLength: 512 title: type: string minLength: 1 @@ -10452,7 +10452,7 @@ components: readOnly: true readable_id: type: string - maxLength: 128 + maxLength: 512 title: type: string maxLength: 256 @@ -10519,7 +10519,7 @@ components: readable_id: type: string minLength: 1 - maxLength: 128 + maxLength: 512 title: type: string minLength: 1 @@ -10951,7 +10951,7 @@ components: readOnly: true readable_id: type: string - maxLength: 128 + maxLength: 512 title: type: string maxLength: 256 @@ -11018,7 +11018,7 @@ components: readable_id: type: string minLength: 1 - maxLength: 128 + maxLength: 512 title: type: string minLength: 1 @@ -11191,7 +11191,7 @@ components: readOnly: true readable_id: type: string - maxLength: 128 + maxLength: 512 title: type: string maxLength: 256 @@ -11258,7 +11258,7 @@ components: readable_id: type: string minLength: 1 - maxLength: 128 + maxLength: 512 title: type: string minLength: 1 From fec303480621fcf61864290f99c8190e5764fcca Mon Sep 17 00:00:00 2001 From: Jon Kafton <939376+jonkafton@users.noreply.github.com> Date: Thu, 11 Jul 2024 18:50:45 +0200 Subject: [PATCH 06/11] Condensed list card components for user lists (#1251) * Condensed list card components. Apply to ItemsListing page * Style lint * Update test and remove duplication --- .../ResourceCard/ResourceCard.tsx | 21 +- .../ListDetailsPage/ItemsListing.test.tsx | 183 +++++++++--------- .../pages/ListDetailsPage/ItemsListing.tsx | 8 +- .../UserListListingPage.tsx | 27 +-- .../src/components/Card/ListCard.tsx | 31 ++- .../src/components/Card/ListCardCondensed.tsx | 113 +++++++++++ .../LearningResourceListCard.tsx | 50 +++-- ...rningResourceListCardCondensed.stories.tsx | 153 +++++++++++++++ .../LearningResourceListCardCondensed.tsx | 157 +++++++++++++++ frontends/ol-components/src/index.ts | 6 +- 10 files changed, 588 insertions(+), 161 deletions(-) create mode 100644 frontends/ol-components/src/components/Card/ListCardCondensed.tsx create mode 100644 frontends/ol-components/src/components/LearningResourceCard/LearningResourceListCardCondensed.stories.tsx create mode 100644 frontends/ol-components/src/components/LearningResourceCard/LearningResourceListCardCondensed.tsx diff --git a/frontends/mit-open/src/page-components/ResourceCard/ResourceCard.tsx b/frontends/mit-open/src/page-components/ResourceCard/ResourceCard.tsx index ce14cb591f..c439cefbf3 100644 --- a/frontends/mit-open/src/page-components/ResourceCard/ResourceCard.tsx +++ b/frontends/mit-open/src/page-components/ResourceCard/ResourceCard.tsx @@ -1,5 +1,9 @@ import React, { useCallback, useMemo, useState } from "react" -import { LearningResourceCard, LearningResourceListCard } from "ol-components" +import { + LearningResourceCard, + LearningResourceListCard, + LearningResourceListCardCondensed, +} from "ol-components" import * as NiceModal from "@ebay/nice-modal-react" import type { LearningResourceCardProps, @@ -104,7 +108,9 @@ const ResourceCard: React.FC = ({ resource, ...others }) => { type ResourceListCardProps = Omit< LearningResourceListCardProps, "href" | "onAddToLearningPathClick" | "onAddToUserListClick" -> +> & { + condensed?: boolean +} /** * Just like `ol-components/LearningResourceListCard`, but with builtin actions: @@ -115,7 +121,8 @@ type ResourceListCardProps = Omit< */ const ResourceListCard: React.FC = ({ resource, - ...others + condensed, + ...props }) => { const { getDrawerHref, @@ -126,16 +133,20 @@ const ResourceListCard: React.FC = ({ inUserList, inLearningPath, } = useResourceCard(resource) + + const ListCardComponent = condensed + ? LearningResourceListCardCondensed + : LearningResourceListCard return ( <> - diff --git a/frontends/mit-open/src/pages/ListDetailsPage/ItemsListing.test.tsx b/frontends/mit-open/src/pages/ListDetailsPage/ItemsListing.test.tsx index f4713b2ba7..d57fe4c3a5 100644 --- a/frontends/mit-open/src/pages/ListDetailsPage/ItemsListing.test.tsx +++ b/frontends/mit-open/src/pages/ListDetailsPage/ItemsListing.test.tsx @@ -30,7 +30,8 @@ jest.mock("ol-components", () => { } }) -const spyLearningResourceCard = jest.mocked(LearningResourceCard) +const spyLearningResourceCardOld = jest.mocked(LearningResourceCard) + const spySortableList = jest.mocked(SortableList) const spySortableItem = jest.mocked(SortableItem) @@ -57,105 +58,107 @@ const getPaginatedRelationships = ( } } -describe.each([ListType.LearningPath, ListType.UserList])( - "ItemsListing", - (listType: string) => { - test("Shows loading message while loading", () => { - const emptyMessage = "Empty list" +describe("ItemsListing", () => { + test.each([ + { listType: ListType.LearningPath }, + { listType: ListType.UserList }, + ])("Shows loading message while loading", ({ listType }) => { + const emptyMessage = "Empty list" + + const { view } = renderWithProviders( + , + ) + screen.getByLabelText("Loading") + view.rerender( + , + ) + }) - const { view } = renderWithProviders( + test.each([ + { listType: ListType.LearningPath, count: 0, hasEmptyMessage: true }, + { + listType: ListType.LearningPath, + count: faker.number.int({ min: 1, max: 5 }), + hasEmptyMessage: false, + }, + { listType: ListType.LearningPath, count: 0, hasEmptyMessage: true }, + { + listType: ListType.UserList, + count: faker.number.int({ min: 1, max: 5 }), + hasEmptyMessage: false, + }, + ])( + "Shows empty message when there are no items", + ({ listType, count, hasEmptyMessage }) => { + setMockResponse.get(urls.userMe.get(), {}) + const emptyMessage = faker.lorem.sentence() + const paginatedRelationships = getPaginatedRelationships( + listType, + count, + faker.number.int(), + ) + renderWithProviders( , ) - screen.getByLabelText("Loading") - view.rerender( - , - ) - }) + const emptyMessageElement = screen.queryByText(emptyMessage) + expect(!!emptyMessageElement).toBe(hasEmptyMessage) + }, + ) - test.each([ - { listType: ListType.LearningPath, count: 0, hasEmptyMessage: true }, - { - listType: ListType.LearningPath, - count: faker.number.int({ min: 1, max: 5 }), - hasEmptyMessage: false, - }, - { listType: ListType.LearningPath, count: 0, hasEmptyMessage: true }, - { - listType: ListType.UserList, - count: faker.number.int({ min: 1, max: 5 }), - hasEmptyMessage: false, - }, - ])( - "Shows empty message when there are no items", - ({ listType, count, hasEmptyMessage }) => { - setMockResponse.get(urls.userMe.get(), {}) - const emptyMessage = faker.lorem.sentence() - const paginatedRelationships = getPaginatedRelationships( - listType, - count, - faker.number.int(), - ) - renderWithProviders( - , - ) - const emptyMessageElement = screen.queryByText(emptyMessage) - expect(!!emptyMessageElement).toBe(hasEmptyMessage) - }, - ) + test.each([ + { listType: ListType.LearningPath, sortable: false, cardProps: {} }, + { + listType: ListType.LearningPath, + sortable: true, + cardProps: { sortable: true }, + }, + { listType: ListType.UserList, sortable: false, cardProps: {} }, + { + listType: ListType.UserList, + sortable: true, + cardProps: { sortable: true }, + }, + ])( + "Shows a list of $listType LearningResourceCards with sortable=$sortable", + ({ listType, sortable, cardProps }) => { + const emptyMessage = faker.lorem.sentence() + const paginatedRelationships = getPaginatedRelationships( + listType, + faker.number.int({ min: 2, max: 4 }), + faker.number.int(), + ) + const items = paginatedRelationships.results as LearningResourceListItem[] + renderWithProviders( + , + { user: {} }, + ) + const titles = items.map((item) => item.resource.title) + const headings = screen.getAllByRole("heading", { + name: (value) => titles.includes(value), + }) + expect(headings.map((h) => h.textContent)).toEqual(titles) - test.each([ - { listType: ListType.LearningPath, sortable: false, cardProps: {} }, - { - listType: ListType.LearningPath, - sortable: true, - cardProps: { sortable: true }, - }, - { listType: ListType.UserList, sortable: false, cardProps: {} }, - { - listType: ListType.UserList, - sortable: true, - cardProps: { sortable: true }, - }, - ])( - "Shows a list of LearningResourceCards with sortable=$sortable", - ({ listType, sortable, cardProps }) => { - const emptyMessage = faker.lorem.sentence() - const paginatedRelationships = getPaginatedRelationships( - listType, - faker.number.int({ min: 2, max: 4 }), - faker.number.int(), - ) - const items = - paginatedRelationships.results as LearningResourceListItem[] - renderWithProviders( - , - { user: {} }, - ) - const titles = items.map((item) => item.resource.title) - const headings = screen.getAllByRole("heading", { - name: (value) => titles.includes(value), - }) - expect(headings.map((h) => h.textContent)).toEqual(titles) + if (sortable) { items.forEach(({ resource }) => { - expectProps(spyLearningResourceCard, { resource, ...cardProps }) + expectProps(spyLearningResourceCardOld, { resource, ...cardProps }) }) - }, - ) - }, -) + } + }, + ) +}) describe.each([ListType.LearningPath, ListType.UserList])( "Sorting ItemListing", diff --git a/frontends/mit-open/src/pages/ListDetailsPage/ItemsListing.tsx b/frontends/mit-open/src/pages/ListDetailsPage/ItemsListing.tsx index 48adf81d7a..b01d0b96ed 100644 --- a/frontends/mit-open/src/pages/ListDetailsPage/ItemsListing.tsx +++ b/frontends/mit-open/src/pages/ListDetailsPage/ItemsListing.tsx @@ -11,6 +11,7 @@ import { styled, PlainList, } from "ol-components" +import { ResourceListCard } from "@/page-components/ResourceCard/ResourceCard" import { useListItemMove } from "api/hooks/learningResources" const EmptyMessage = styled.p({ @@ -38,14 +39,11 @@ const ItemsListingViewOnly: React.FC<{ items: NonNullable }> = ({ items }) => { return ( - + {items.map((item) => { return (
  • - +
    • ) })} diff --git a/frontends/mit-open/src/pages/UserListListingPage/UserListListingPage.tsx b/frontends/mit-open/src/pages/UserListListingPage/UserListListingPage.tsx index ad101d9aca..1b5e85ddc9 100644 --- a/frontends/mit-open/src/pages/UserListListingPage/UserListListingPage.tsx +++ b/frontends/mit-open/src/pages/UserListListingPage/UserListListingPage.tsx @@ -73,24 +73,6 @@ const EditUserListMenu: React.FC = ({ userList }) => { ) } -type ListCardProps = { - list: UserList - onActivate: (userList: UserList) => void - canEdit: boolean -} -const ListCard: React.FC = ({ list, onActivate }) => { - return ( - } - /> - ) -} - type UserListListingComponentProps = { title?: string onActivate: (userList: UserList) => void @@ -132,10 +114,13 @@ const UserListListingComponent: React.FC = ( {listingQuery.data.results?.map((list) => { return (
  • - } />
    • ) diff --git a/frontends/ol-components/src/components/Card/ListCard.tsx b/frontends/ol-components/src/components/Card/ListCard.tsx index 4650a944fb..40ff2a3bdc 100644 --- a/frontends/ol-components/src/components/Card/ListCard.tsx +++ b/frontends/ol-components/src/components/Card/ListCard.tsx @@ -12,7 +12,7 @@ import { Wrapper, containerStyles } from "./Card" import { TruncateText } from "../TruncateText/TruncateText" import { ActionButton, ActionButtonProps } from "../Button/Button" -const LinkContainer = styled(Link)` +export const LinkContainer = styled(Link)` ${containerStyles} display: flex; @@ -26,13 +26,13 @@ const LinkContainer = styled(Link)` } ` -const Container = styled.div` +export const Container = styled.div` ${containerStyles} ` const Content = () => <> -const Body = styled.div` +export const Body = styled.div` flex-grow: 1; overflow: hidden; margin: 24px; @@ -63,7 +63,7 @@ const Image = styled.img` flex-shrink: 0; ` -const Info = styled.div` +export const Info = styled.div` ${{ ...theme.typography.subtitle3 }} margin-bottom: 16px; ${theme.breakpoints.down("md")} { @@ -77,35 +77,28 @@ const Info = styled.div` align-items: center; ` -const Title = styled.h3` +export const Title = styled.h3` flex-grow: 1; color: ${theme.custom.colors.darkGray2}; text-overflow: ellipsis; ${{ ...theme.typography.subtitle1 }} height: ${theme.typography.pxToRem(40)}; ${theme.breakpoints.down("md")} { - ${{ ...theme.typography.subtitle3 }} + ${{ ...theme.typography.subtitle2 }} height: ${theme.typography.pxToRem(32)}; } margin: 0; ` -const Footer = styled.span` +export const Footer = styled.span` display: block; - ${{ - ...theme.typography.body3, - color: theme.custom.colors.silverGrayDark, - }} - - span { - color: ${theme.custom.colors.black}; - } - + ${{ ...theme.typography.body3 }} + color: ${theme.custom.colors.darkGray2}; white-space: nowrap; ` -const Bottom = styled.div` +export const Bottom = styled.div` display: flex; justify-content: space-between; align-items: flex-end; @@ -118,7 +111,7 @@ const Bottom = styled.div` /** * Slot intended to contain ListCardAction buttons. */ -const Actions = styled.div<{ hasImage?: boolean }>` +export const Actions = styled.div<{ hasImage?: boolean }>` display: flex; gap: 8px; position: absolute; @@ -152,7 +145,7 @@ type CardProps = { className?: string href?: string } -type Card = FC & { +export type Card = FC & { Content: FC<{ children: ReactNode }> Image: FC> Info: FC<{ children: ReactNode }> diff --git a/frontends/ol-components/src/components/Card/ListCardCondensed.tsx b/frontends/ol-components/src/components/Card/ListCardCondensed.tsx new file mode 100644 index 0000000000..219fb64e0e --- /dev/null +++ b/frontends/ol-components/src/components/Card/ListCardCondensed.tsx @@ -0,0 +1,113 @@ +import React, { FC, ReactNode, Children, isValidElement } from "react" +import styled from "@emotion/styled" +import { theme } from "../ThemeProvider/ThemeProvider" +import { Wrapper } from "./Card" +import { TruncateText } from "../TruncateText/TruncateText" +import { + ListCard, + Body as BaseBody, + LinkContainer, + Container, + Info as BaseInfo, + Title as BaseTitle, + Footer, + Actions as BaseActions, + Bottom as BaseBottom, +} from "./ListCard" +import type { Card as BaseCard } from "./ListCard" + +const Body = styled(BaseBody)` + margin: 16px; + ${theme.breakpoints.down("md")} { + margin: 16px; + } +` + +const Info = styled(BaseInfo)` + margin-bottom: 4px; +` + +const Title = styled(BaseTitle)` + height: auto; + margin-bottom: 8px; + margin-right: 82px; + ${theme.breakpoints.down("md")} { + height: auto; + ${{ ...theme.typography.subtitle2 }} + } +` + +const Bottom = styled(BaseBottom)` + height: auto; + ${theme.breakpoints.down("md")} { + height: auto; + } +` +export const Actions = styled(BaseActions)` + bottom: 16px; + right: 16px; + gap: 16px; + ${theme.breakpoints.down("md")} { + bottom: 16px; + right: 16px; + gap: 16px; + } +` +const Content = () => <> + +type CardProps = { + children: ReactNode[] | ReactNode + className?: string + href?: string +} + +type Card = FC & Omit + +const ListCardCondensed: Card = ({ children, className, href }) => { + const _Container = href ? LinkContainer : Container + + let content, info, title, footer, actions + + Children.forEach(children, (child) => { + if (!isValidElement(child)) return + if (child.type === Content) content = child.props.children + else if (child.type === Info) info = child.props.children + else if (child.type === Title) title = child.props.children + else if (child.type === Footer) footer = child.props.children + else if (child.type === Actions) actions = child.props.children + }) + + if (content) { + return ( + <_Container className={className} to={href!}> + {content} + + ) + } + + return ( + + <_Container to={href!}> + + {info} + + <TruncateText lineClamp={4}>{title}</TruncateText> + + +
    {footer}
    +     + + + {actions && {actions}} +     + ) +} + +ListCardCondensed.Content = Content +ListCardCondensed.Info = Info +ListCardCondensed.Title = Title +ListCardCondensed.Footer = Footer +ListCardCondensed.Actions = Actions +ListCardCondensed.Action = ListCard.Action + +export { ListCardCondensed } diff --git a/frontends/ol-components/src/components/LearningResourceCard/LearningResourceListCard.tsx b/frontends/ol-components/src/components/LearningResourceCard/LearningResourceListCard.tsx index c16a0a09ea..4bce3c552a 100644 --- a/frontends/ol-components/src/components/LearningResourceCard/LearningResourceListCard.tsx +++ b/frontends/ol-components/src/components/LearningResourceCard/LearningResourceListCard.tsx @@ -26,13 +26,14 @@ const IMAGE_SIZES = { desktop: { width: 236, height: 122 }, } -const CardLabel = styled.span` +export const CardLabel = styled.span` + color: ${theme.custom.colors.silverGrayDark}; ${theme.breakpoints.down("sm")} { display: none; } ` -const Certificate = styled.div` +export const Certificate = styled.div` border-radius: 4px; background-color: ${theme.custom.colors.lightGray1}; padding: 4px; @@ -65,7 +66,7 @@ const Certificate = styled.div` align-items: center; ` -const Price = styled.div` +export const Price = styled.div` ${{ ...theme.typography.subtitle2 }} color: ${theme.custom.colors.darkGray2}; ${theme.breakpoints.down("md")} { @@ -73,7 +74,7 @@ const Price = styled.div` } ` -const BorderSeparator = styled.div` +export const BorderSeparator = styled.div` div { display: inline; } @@ -102,7 +103,7 @@ type Prices = { certificate: null | number } -const getPrices = (resource: LearningResource) => { +export const getPrices = (resource: LearningResource) => { const prices: Prices = { course: null, certificate: null, @@ -198,7 +199,7 @@ const getDisplayPrecision = (price: number) => { return price.toFixed(2) } -const getDisplayPrice = (price: number | number[] | null) => { +export const getDisplayPrice = (price: number | number[] | null) => { if (price === null) { return null } @@ -221,7 +222,6 @@ const getDisplayPrice = (price: number | number[] | null) => { */ const Info = ({ resource }: { resource: LearningResource }) => { const prices = getPrices(resource) - getDisplayPrice(+Infinity) return ( <> {getReadableResourceType(resource.resource_type)} @@ -237,7 +237,7 @@ const Info = ({ resource }: { resource: LearningResource }) => { ) } -const Count = ({ resource }: { resource: LearningResource }) => { +export const Count = ({ resource }: { resource: LearningResource }) => { if (resource.resource_type !== ResourceTypeEnum.LearningPath) { return null } @@ -270,7 +270,9 @@ const getStartDate = (resource: LearningResource) => { return formatDate(startDate, "MMMM DD, YYYY") } -const StartDate: React.FC<{ resource: LearningResource }> = ({ resource }) => { +export const StartDate: React.FC<{ resource: LearningResource }> = ({ + resource, +}) => { const startDate = getStartDate(resource) if (!startDate) return null @@ -284,7 +286,7 @@ const StartDate: React.FC<{ resource: LearningResource }> = ({ resource }) => { ) } -const Format = ({ resource }: { resource: LearningResource }) => { +export const Format = ({ resource }: { resource: LearningResource }) => { const format = resource.learning_format?.[0]?.name if (!format) return null return ( @@ -356,18 +358,26 @@ interface LearningResourceListCardProps { inLearningPath?: boolean } -const FILLED_PROPS = { variant: "primary" } as const -const UNFILLED_PROPS = { color: "secondary", variant: "secondary" } as const -const CardActionButton: React.FC< - Pick & { - filled?: boolean - isMobile?: boolean - } -> = ({ filled, isMobile, ...props }) => { +type CardActionButtonProps = Pick< + ActionButtonProps, + "aria-label" | "onClick" | "children" +> & { + filled?: boolean + isMobile?: boolean +} + +export const CardActionButton: React.FC = ({ + filled, + isMobile, + ...props +}) => { + const FILLED_PROPS = { variant: "primary" } as const + const UNFILLED_PROPS = { color: "secondary", variant: "secondary" } as const + return ( @@ -435,10 +445,12 @@ const LearningResourceListCard: React.FC = ({ {editMenu} + export{" "} + export{" "} diff --git a/frontends/ol-components/src/components/LearningResourceCard/LearningResourceListCardCondensed.stories.tsx b/frontends/ol-components/src/components/LearningResourceCard/LearningResourceListCardCondensed.stories.tsx new file mode 100644 index 0000000000..4ec396c2d6 --- /dev/null +++ b/frontends/ol-components/src/components/LearningResourceCard/LearningResourceListCardCondensed.stories.tsx @@ -0,0 +1,153 @@ +import React from "react" +import type { Meta, StoryObj } from "@storybook/react" +import { LearningResourceListCardCondensed } from "./LearningResourceListCardCondensed" +import { ResourceTypeEnum } from "api" +import { factories } from "api/test-utils" +import { withRouter } from "storybook-addon-react-router-v6" + +const _makeResource = factories.learningResources.resource + +const makeResource: typeof _makeResource = (overrides) => { + const resource = _makeResource(overrides) + resource.image!.url = + "https://ocw.mit.edu/courses/res-hso-001-mit-haystack-observatory-k12-stem-lesson-plans/mitres_hso_001.jpg" + return resource +} + +const meta: Meta = { + title: "smoot-design/Cards/LearningResourceListCardCondensed", + argTypes: { + resource: { + options: ["Loading", ...Object.values(ResourceTypeEnum)], + mapping: { + Loading: undefined, + [ResourceTypeEnum.Course]: makeResource({ + resource_type: ResourceTypeEnum.Course, + }), + [ResourceTypeEnum.Program]: makeResource({ + resource_type: ResourceTypeEnum.Program, + }), + [ResourceTypeEnum.Video]: makeResource({ + resource_type: ResourceTypeEnum.Video, + url: "https://www.youtube.com/watch?v=-E9hf5RShzQ", + }), + [ResourceTypeEnum.VideoPlaylist]: makeResource({ + resource_type: ResourceTypeEnum.VideoPlaylist, + }), + [ResourceTypeEnum.Podcast]: makeResource({ + resource_type: ResourceTypeEnum.Podcast, + }), + [ResourceTypeEnum.PodcastEpisode]: makeResource({ + resource_type: ResourceTypeEnum.PodcastEpisode, + }), + [ResourceTypeEnum.LearningPath]: makeResource({ + resource_type: ResourceTypeEnum.LearningPath, + }), + }, + }, + onAddToLearningPathClick: { + action: "click-add-to-learning-path", + }, + onAddToUserListClick: { + action: "click-add-to-user-list", + }, + }, + render: ({ + resource, + isLoading, + onAddToLearningPathClick, + onAddToUserListClick, + }) => ( + + ), + decorators: [withRouter], +} + +export default meta + +type Story = StoryObj + +export const PaidCourse: Story = { + args: { + resource: makeResource({ + resource_type: ResourceTypeEnum.Course, + runs: [factories.learningResources.run()], + free: false, + certification: true, + prices: ["999"], + }), + }, +} + +export const FreeCourse: Story = { + args: { + resource: makeResource({ + resource_type: ResourceTypeEnum.Course, + runs: [factories.learningResources.run()], + free: true, + certification: true, + prices: ["0", "400"], + }), + }, +} + +export const LearningPath: Story = { + args: { + resource: makeResource({ resource_type: ResourceTypeEnum.LearningPath }), + }, +} + +export const Program: Story = { + args: { + resource: makeResource({ resource_type: ResourceTypeEnum.Program }), + }, +} + +export const Podcast: Story = { + args: { + resource: makeResource({ + resource_type: ResourceTypeEnum.Podcast, + free: true, + }), + }, +} + +export const PodcastEpisode: Story = { + args: { + resource: makeResource({ + resource_type: ResourceTypeEnum.PodcastEpisode, + free: true, + }), + }, +} + +export const Video: Story = { + args: { + resource: makeResource({ + resource_type: ResourceTypeEnum.Video, + url: "https://www.youtube.com/watch?v=4A9bGL-_ilA", + free: true, + }), + }, +} + +export const VideoPlaylist: Story = { + args: { + resource: makeResource({ + resource_type: ResourceTypeEnum.VideoPlaylist, + free: true, + }), + }, +} + +export const Loading: Story = { + args: { + isLoading: true, + }, +} diff --git a/frontends/ol-components/src/components/LearningResourceCard/LearningResourceListCardCondensed.tsx b/frontends/ol-components/src/components/LearningResourceCard/LearningResourceListCardCondensed.tsx new file mode 100644 index 0000000000..efb37027cd --- /dev/null +++ b/frontends/ol-components/src/components/LearningResourceCard/LearningResourceListCardCondensed.tsx @@ -0,0 +1,157 @@ +import React from "react" +import styled from "@emotion/styled" +import Skeleton from "@mui/material/Skeleton" +import { + RiMenuAddLine, + RiBookmarkLine, + RiAwardFill, + RiBookmarkFill, +} from "@remixicon/react" +import { LearningResource } from "api" +import { getReadableResourceType } from "ol-utilities" +import { ListCardCondensed } from "../Card/ListCardCondensed" +import { useMuiBreakpointAtLeast } from "../../hooks/useBreakpoint" +import { + Certificate, + Price, + BorderSeparator, + getPrices, + getDisplayPrice, + Count, + StartDate, + Format, +} from "./LearningResourceListCard" +import type { LearningResourceListCardProps } from "./LearningResourceListCard" +import { ActionButton, ActionButtonProps } from "../Button/Button" + +const ResourceType = styled.span` + align-self: flex-start; +` + +/* The only variation on the LearningResourceListCard + * Info is the ResourceType flex alignment + */ +const Info = ({ resource }: { resource: LearningResource }) => { + const prices = getPrices(resource) + return ( + <> + + {getReadableResourceType(resource.resource_type)} + + {resource.certification && ( + + + Certificate{prices?.certificate ? ":" : ""}{" "} + {getDisplayPrice(prices?.certificate)} + + )} + {getDisplayPrice(prices?.course)} + + ) +} + +const Loading = styled.div<{ mobile?: boolean }>` + padding: 16px; +` + +const LoadingView = ({ isMobile }: { isMobile: boolean }) => { + return ( + + + + + + ) +} + +type CardActionButtonProps = Pick< + ActionButtonProps, + "aria-label" | "onClick" | "children" +> & { + filled?: boolean +} + +export const CardActionButton: React.FC = ({ + filled, + ...props +}) => { + const FILLED_PROPS = { variant: "primary" } as const + const UNFILLED_PROPS = { color: "secondary", variant: "secondary" } as const + + return ( + + ) +} + +const LearningResourceListCardCondensed: React.FC< + LearningResourceListCardProps +> = ({ + isLoading, + resource, + className, + href, + onAddToLearningPathClick, + onAddToUserListClick, + editMenu, + inLearningPath, + inUserList, +}) => { + const isMobile = !useMuiBreakpointAtLeast("md") + + if (isLoading) { + return ( + + + + + + ) + } + if (!resource) { + return null + } + return ( + + + + + {resource.title} + + {onAddToLearningPathClick && ( + onAddToLearningPathClick(event, resource.id)} + > + + + )} + {onAddToUserListClick && ( + onAddToUserListClick(event, resource.id)} + > + {inUserList ? : } + + )} + {editMenu} + + + + + + + + + + ) +} + +export { LearningResourceListCardCondensed } +export type { LearningResourceListCardProps as LearningResourceListCardCondensedProps } diff --git a/frontends/ol-components/src/index.ts b/frontends/ol-components/src/index.ts index 9913ff4a29..fb0ffb53af 100644 --- a/frontends/ol-components/src/index.ts +++ b/frontends/ol-components/src/index.ts @@ -28,7 +28,7 @@ export { ActionButtonLink, ButtonLink, } from "./components/Button/Button" -export { ListCardActionButton } from "./components/Card/ListCard" +export { ListCard, ListCardActionButton } from "./components/Card/ListCard" export type { ButtonProps, @@ -167,7 +167,9 @@ export * from "./components/Chips/ChipLink" export * from "./components/EmbedlyCard/EmbedlyCard" export * from "./components/FormDialog/FormDialog" export * from "./components/LearningResourceCard/LearningResourceCard" -export * from "./components/LearningResourceCard/LearningResourceListCard" +export { LearningResourceListCard } from "./components/LearningResourceCard/LearningResourceListCard" +export type { LearningResourceListCardProps } from "./components/LearningResourceCard/LearningResourceListCard" +export * from "./components/LearningResourceCard/LearningResourceListCardCondensed" export * from "./components/LearningResourceExpanded/LearningResourceExpanded" export * from "./components/LoadingSpinner/LoadingSpinner" export * from "./components/Logo/Logo" From 6c65b1e7a332b6c8981aaff14f18434e60049746 Mon Sep 17 00:00:00 2001 From: Matt Bertrand Date: Thu, 11 Jul 2024 12:55:29 -0400 Subject: [PATCH 07/11] Plain text news/events titles/authors; standardize html cleanup (#1248) --- learning_resources/etl/constants.py | 30 ----------------------- learning_resources/etl/mitxonline.py | 2 +- learning_resources/etl/mitxonline_test.py | 2 +- learning_resources/etl/ocw.py | 2 +- learning_resources/etl/ocw_test.py | 3 +-- learning_resources/etl/openedx.py | 2 +- learning_resources/etl/podcast.py | 3 +-- learning_resources/etl/prolearn.py | 4 +-- learning_resources/etl/prolearn_test.py | 3 ++- learning_resources/etl/utils.py | 12 --------- learning_resources/etl/utils_test.py | 29 ---------------------- learning_resources/etl/xpro.py | 2 +- learning_resources/etl/youtube.py | 3 +-- learning_resources/etl/youtube_test.py | 2 +- main/constants.py | 27 ++++++++++++++++++++ main/utils.py | 12 +++++++++ main/utils_test.py | 30 +++++++++++++++++++++++ news_events/etl/medium_mit_news.py | 5 ++-- news_events/etl/medium_mit_news_test.py | 4 +-- news_events/etl/ol_events.py | 7 +++--- news_events/etl/sloan_exec_news.py | 26 +++++++++----------- news_events/etl/sloan_exec_news_test.py | 14 ++++++++++- news_events/etl/utils.py | 20 --------------- news_events/etl/utils_test.py | 10 -------- test_html/test_medium_mit_news.rss | 2 +- test_json/ol_events_output.json | 4 +-- test_json/test_sloan_exec_news.json | 10 ++++---- 27 files changed, 123 insertions(+), 147 deletions(-) diff --git a/learning_resources/etl/constants.py b/learning_resources/etl/constants.py index 825f0f89b8..f3dffd1c8d 100644 --- a/learning_resources/etl/constants.py +++ b/learning_resources/etl/constants.py @@ -80,33 +80,3 @@ class CourseNumberType(Enum): for value in LearningResourceFormat.values() }, } - - -ALLOWED_HTML_TAGS = { - "b", - "blockquote", - "br", - "caption", - "center", - "cite", - "code", - "div", - "em", - "hr", - "i", - "li", - "ol", - "p", - "pre", - "q", - "small", - "span", - "strike", - "strong", - "sub", - "sup", - "u", - "ul", -} - -ALLOWED_HTML_ATTRIBUTES = {} diff --git a/learning_resources/etl/mitxonline.py b/learning_resources/etl/mitxonline.py index 2e9c71b0b5..f21981fc87 100644 --- a/learning_resources/etl/mitxonline.py +++ b/learning_resources/etl/mitxonline.py @@ -19,12 +19,12 @@ ) from learning_resources.etl.constants import ETLSource from learning_resources.etl.utils import ( - clean_data, extract_valid_department_from_id, generate_course_numbers_json, parse_certification, transform_topics, ) +from main.utils import clean_data log = logging.getLogger(__name__) diff --git a/learning_resources/etl/mitxonline_test.py b/learning_resources/etl/mitxonline_test.py index c7db475ba1..746a41a8dc 100644 --- a/learning_resources/etl/mitxonline_test.py +++ b/learning_resources/etl/mitxonline_test.py @@ -31,11 +31,11 @@ ) from learning_resources.etl.utils import ( UCC_TOPIC_MAPPINGS, - clean_data, extract_valid_department_from_id, parse_certification, ) from main.test_utils import any_instance_of +from main.utils import clean_data pytestmark = pytest.mark.django_db diff --git a/learning_resources/etl/ocw.py b/learning_resources/etl/ocw.py index e179c8cfa5..e71db82f99 100644 --- a/learning_resources/etl/ocw.py +++ b/learning_resources/etl/ocw.py @@ -26,7 +26,6 @@ ) from learning_resources.etl.constants import ETLSource from learning_resources.etl.utils import ( - clean_data, extract_text_metadata, generate_course_numbers_json, get_content_type, @@ -38,6 +37,7 @@ parse_instructors, safe_load_json, ) +from main.utils import clean_data log = logging.getLogger(__name__) diff --git a/learning_resources/etl/ocw_test.py b/learning_resources/etl/ocw_test.py index df9a436a1b..08896026ee 100644 --- a/learning_resources/etl/ocw_test.py +++ b/learning_resources/etl/ocw_test.py @@ -16,14 +16,13 @@ transform_contentfile, transform_course, ) -from learning_resources.etl.utils import clean_data from learning_resources.factories import ContentFileFactory from learning_resources.models import ContentFile from learning_resources.utils import ( get_s3_object_and_read, safe_load_json, ) -from main.utils import now_in_utc +from main.utils import clean_data, now_in_utc pytestmark = pytest.mark.django_db diff --git a/learning_resources/etl/openedx.py b/learning_resources/etl/openedx.py index 4e772ca6e4..112126a904 100644 --- a/learning_resources/etl/openedx.py +++ b/learning_resources/etl/openedx.py @@ -17,7 +17,6 @@ ) from learning_resources.etl.constants import COMMON_HEADERS from learning_resources.etl.utils import ( - clean_data, extract_valid_department_from_id, generate_course_numbers_json, parse_certification, @@ -25,6 +24,7 @@ without_none, ) from learning_resources.utils import get_year_and_semester +from main.utils import clean_data MIT_OWNER_KEYS = ["MITx", "MITx_PRO"] diff --git a/learning_resources/etl/podcast.py b/learning_resources/etl/podcast.py index a6e40379a8..cd466443c0 100644 --- a/learning_resources/etl/podcast.py +++ b/learning_resources/etl/podcast.py @@ -12,9 +12,8 @@ from learning_resources.constants import LearningResourceType from learning_resources.etl.constants import ETLSource -from learning_resources.etl.utils import clean_data from learning_resources.models import PodcastEpisode -from main.utils import frontend_absolute_url, now_in_utc +from main.utils import clean_data, frontend_absolute_url, now_in_utc CONFIG_FILE_REPO = "mitodl/open-podcast-data" CONFIG_FILE_FOLDER = "podcasts" diff --git a/learning_resources/etl/prolearn.py b/learning_resources/etl/prolearn.py index a01cdc008f..a91a0dbe4c 100644 --- a/learning_resources/etl/prolearn.py +++ b/learning_resources/etl/prolearn.py @@ -11,9 +11,9 @@ from learning_resources.constants import CertificationType from learning_resources.etl.constants import ETLSource -from learning_resources.etl.utils import clean_data, transform_format, transform_topics +from learning_resources.etl.utils import transform_format, transform_topics from learning_resources.models import LearningResourceOfferor, LearningResourcePlatform -from main.utils import now_in_utc +from main.utils import clean_data, now_in_utc log = logging.getLogger(__name__) diff --git a/learning_resources/etl/prolearn_test.py b/learning_resources/etl/prolearn_test.py index 6c1266de60..d4e0d2dea1 100644 --- a/learning_resources/etl/prolearn_test.py +++ b/learning_resources/etl/prolearn_test.py @@ -29,13 +29,14 @@ transform_programs, update_format, ) -from learning_resources.etl.utils import clean_data, transform_format +from learning_resources.etl.utils import transform_format from learning_resources.factories import ( LearningResourceOfferorFactory, LearningResourcePlatformFactory, ) from learning_resources.models import LearningResourceOfferor, LearningResourcePlatform from main.test_utils import assert_json_equal +from main.utils import clean_data pytestmark = pytest.mark.django_db diff --git a/learning_resources/etl/utils.py b/learning_resources/etl/utils.py index c7257827af..11c2e283fd 100644 --- a/learning_resources/etl/utils.py +++ b/learning_resources/etl/utils.py @@ -23,7 +23,6 @@ from django.conf import settings from django.utils.functional import SimpleLazyObject from django.utils.text import slugify -from nh3 import nh3 from tika import parser as tika_parser from xbundle import XBundle @@ -40,8 +39,6 @@ OfferedBy, ) from learning_resources.etl.constants import ( - ALLOWED_HTML_ATTRIBUTES, - ALLOWED_HTML_TAGS, RESOURCE_FORMAT_MAPPING, CourseNumberType, ETLSource, @@ -692,12 +689,3 @@ def parse_certification(offeror, runs_data): if (availability and availability != AvailabilityType.archived.value) ] ) - - -def clean_data(data: str) -> str: - """Remove unwanted html tags from text""" - if data: - return nh3.clean( - data, tags=ALLOWED_HTML_TAGS, attributes=ALLOWED_HTML_ATTRIBUTES - ) - return "" diff --git a/learning_resources/etl/utils_test.py b/learning_resources/etl/utils_test.py index a99c7b7229..4a37ff491d 100644 --- a/learning_resources/etl/utils_test.py +++ b/learning_resources/etl/utils_test.py @@ -435,35 +435,6 @@ def test_parse_certification(offered_by, availability, has_cert): ) -@pytest.mark.parametrize( - ("input_text", "output_text"), - [ - ("The cat sat on the mat & spat.\n", "The cat sat on the mat & spat.\n"), - ( - "the dog chased a hog", - "the dog chased a hog", - ), - ( - "

    What a mess

    ", - "

    What a mess

    ", - ), - ( - "

    Some text

    ", - "

    Some text

    ", - ), - ( - "

    Hello, world!

    ", - "

    Hello, world!

    ", - ), - (None, ""), - ("", ""), - ], -) -def test_clean_data(input_text, output_text): - """clean_data function should return expected output""" - assert utils.clean_data(input_text) == output_text - - @pytest.mark.parametrize( ("previous_archive", "identical"), [ diff --git a/learning_resources/etl/xpro.py b/learning_resources/etl/xpro.py index 88c515e458..24fd4de42d 100644 --- a/learning_resources/etl/xpro.py +++ b/learning_resources/etl/xpro.py @@ -16,11 +16,11 @@ ) from learning_resources.etl.constants import ETLSource from learning_resources.etl.utils import ( - clean_data, generate_course_numbers_json, transform_format, transform_topics, ) +from main.utils import clean_data log = logging.getLogger(__name__) diff --git a/learning_resources/etl/youtube.py b/learning_resources/etl/youtube.py index f9a8334b58..869a70946d 100644 --- a/learning_resources/etl/youtube.py +++ b/learning_resources/etl/youtube.py @@ -22,9 +22,8 @@ from learning_resources.etl.constants import ETLSource from learning_resources.etl.exceptions import ExtractException from learning_resources.etl.loaders import update_index -from learning_resources.etl.utils import clean_data from learning_resources.models import LearningResource -from main.utils import now_in_utc +from main.utils import clean_data, now_in_utc CONFIG_FILE_REPO = "mitodl/open-video-data" CONFIG_FILE_FOLDER = "youtube" diff --git a/learning_resources/etl/youtube_test.py b/learning_resources/etl/youtube_test.py index 4f5f6d8cee..7c2d21172a 100644 --- a/learning_resources/etl/youtube_test.py +++ b/learning_resources/etl/youtube_test.py @@ -16,8 +16,8 @@ from learning_resources.etl import youtube from learning_resources.etl.constants import ETLSource from learning_resources.etl.exceptions import ExtractException -from learning_resources.etl.utils import clean_data from learning_resources.factories import VideoFactory +from main.utils import clean_data @pytest.fixture() diff --git a/main/constants.py b/main/constants.py index f35f4dc4e7..a9449dc8b2 100644 --- a/main/constants.py +++ b/main/constants.py @@ -11,3 +11,30 @@ ISOFORMAT = "%Y-%m-%dT%H:%M:%SZ" VALID_HTTP_METHODS = ["get", "post", "patch", "delete"] +ALLOWED_HTML_TAGS = { + "b", + "blockquote", + "br", + "caption", + "center", + "cite", + "code", + "div", + "em", + "hr", + "i", + "li", + "ol", + "p", + "pre", + "q", + "small", + "span", + "strike", + "strong", + "sub", + "sup", + "u", + "ul", +} +ALLOWED_HTML_ATTRIBUTES = {} diff --git a/main/utils.py b/main/utils.py index 0a36fe6e4a..2f8862a6b1 100644 --- a/main/utils.py +++ b/main/utils.py @@ -10,6 +10,9 @@ import markdown2 from bs4 import BeautifulSoup from django.conf import settings +from nh3 import nh3 + +from main.constants import ALLOWED_HTML_ATTRIBUTES, ALLOWED_HTML_TAGS log = logging.getLogger(__name__) @@ -310,3 +313,12 @@ def frontend_absolute_url(relative_path: str) -> str: str: absolute url path to the frontend """ return urljoin(settings.APP_BASE_URL, relative_path) + + +def clean_data(data: str) -> str: + """Remove unwanted html tags from text""" + if data: + return nh3.clean( + data, tags=ALLOWED_HTML_TAGS, attributes=ALLOWED_HTML_ATTRIBUTES + ) + return "" diff --git a/main/utils_test.py b/main/utils_test.py index 8a6b3e74b0..c495640e7d 100644 --- a/main/utils_test.py +++ b/main/utils_test.py @@ -10,6 +10,7 @@ from main.factories import UserFactory from main.utils import ( chunks, + clean_data, extract_values, filter_dict_keys, filter_dict_with_renamed_keys, @@ -211,3 +212,32 @@ def test_frontend_absolute_url(settings): assert frontend_absolute_url("/") == "http://example.com/" assert frontend_absolute_url("/path") == "http://example.com/path" assert frontend_absolute_url("path") == "http://example.com/path" + + +@pytest.mark.parametrize( + ("input_text", "output_text"), + [ + ("The cat sat on the mat & spat.\n", "The cat sat on the mat & spat.\n"), + ( + "the dog chased a hog", + "the dog chased a hog", + ), + ( + "

    What a mess

    ", + "

    What a mess

    ", + ), + ( + "

    Some text

    ", + "

    Some text

    ", + ), + ( + "

    Hello, world!

    ", + "

    Hello, world!

    ", + ), + (None, ""), + ("", ""), + ], +) +def test_clean_data(input_text, output_text): + """clean_data function should return expected output""" + assert clean_data(input_text) == output_text diff --git a/news_events/etl/medium_mit_news.py b/news_events/etl/medium_mit_news.py index 1b977b6478..5f686083cc 100644 --- a/news_events/etl/medium_mit_news.py +++ b/news_events/etl/medium_mit_news.py @@ -3,6 +3,7 @@ import feedparser from bs4 import BeautifulSoup +from main.utils import clean_data from news_events.constants import FeedType from news_events.etl.utils import stringify_time_struct @@ -65,8 +66,8 @@ def transform_items(items_data: list[dict]) -> list[dict]: "guid": item.get("id"), "title": item.get("title", ""), "url": item.get("link", None), - "summary": item.get("summary", ""), - "content": content, + "summary": clean_data(item.get("summary", "")), + "content": clean_data(content), "image": image_data, "detail": { "authors": [ diff --git a/news_events/etl/medium_mit_news_test.py b/news_events/etl/medium_mit_news_test.py index db77ac80ea..203577de55 100644 --- a/news_events/etl/medium_mit_news_test.py +++ b/news_events/etl/medium_mit_news_test.py @@ -71,8 +71,8 @@ def test_transform_items(mocker, medium_mit_rss_data): "guid": "https://medium.com/p/a685e2b89c6a", "title": "Meet 8 MIT women faculty who teach MITx courses and lead cutting-edge research", "url": "https://medium.com/open-learning/meet-8-mit-women-faculty", - "summary": "

    Celebrating Women\u2019s History Month with MIT women\u2019s truncated

    ", - "content": "

    Celebrating Women\u2019s History Month with MIT women\u2019s truncated

    ", + "summary": "Celebrating Women\u2019s History Month with MIT women\u2019s truncated", + "content": "Celebrating Women\u2019s History Month with MIT women\u2019s truncated", "image": None, "detail": { "authors": ["MIT Open Learning"], diff --git a/news_events/etl/ol_events.py b/news_events/etl/ol_events.py index ca6fcb15ab..36510ea627 100644 --- a/news_events/etl/ol_events.py +++ b/news_events/etl/ol_events.py @@ -6,9 +6,8 @@ from zoneinfo import ZoneInfo from dateutil import parser -from django.utils.html import strip_tags -from main.utils import now_in_utc +from main.utils import clean_data, now_in_utc from news_events.constants import FeedType from news_events.etl.utils import get_request_json @@ -173,8 +172,8 @@ def transform_event(event_data: dict) -> dict or None: event_data.get("relationships", {}).get("field_event_image", {}) ) ), - "summary": strip_tags(attributes.get("body", {}).get("value") or ""), - "content": strip_tags(attributes.get("body", {}).get("value") or ""), + "summary": clean_data(attributes.get("body", {}).get("value") or ""), + "content": clean_data(attributes.get("body", {}).get("value") or ""), "detail": { "location": transform_relationship( extract_relationship(event_data, "field_location_tag") diff --git a/news_events/etl/sloan_exec_news.py b/news_events/etl/sloan_exec_news.py index 336bd0043a..6b3e2d3fe0 100644 --- a/news_events/etl/sloan_exec_news.py +++ b/news_events/etl/sloan_exec_news.py @@ -6,10 +6,9 @@ from urllib.parse import urlencode, urljoin import requests -from bs4 import BeautifulSoup as Soup +from main.utils import clean_data from news_events.constants import FeedType -from news_events.etl.utils import tag_text log = logging.getLogger(__name__) @@ -96,19 +95,18 @@ def transform_item(item_data: dict) -> dict: """ return { "guid": item_data.get("managedContentId"), - "title": html.escape(item_data.get("title", "")), - "summary": tag_text( - Soup( - html.unescape( - item_data.get("contentNodes", {}).get("Summary", {}).get("value") - ), - "lxml", + "title": html.unescape(item_data.get("title", "")), + "summary": clean_data( + html.unescape( + item_data.get("contentNodes", {}).get("Summary", {}).get("value") ) ), - "content": html.unescape( - item_data.get("contentNodes", {}) - .get("First_Section_Content", {}) - .get("value") + "content": clean_data( + html.unescape( + item_data.get("contentNodes", {}) + .get("First_Section_Content", {}) + .get("value") + ) ), "url": urljoin( SLOAN_EXEC_ARTICLE_PREFIX_URL, @@ -131,7 +129,7 @@ def transform_item(item_data: dict) -> dict: }, "detail": { "authors": [ - html.escape( + html.unescape( item_data.get("contentNodes", {}) .get("Quote_Author", {}) .get("value", "") diff --git a/news_events/etl/sloan_exec_news_test.py b/news_events/etl/sloan_exec_news_test.py index 7a8bf06951..66201902cc 100644 --- a/news_events/etl/sloan_exec_news_test.py +++ b/news_events/etl/sloan_exec_news_test.py @@ -43,7 +43,12 @@ def test_transform(): items = list(source["items"]) assert len(items) == 20 assert items[0]["detail"]["publish_date"] == "2024-03-25T16:29:25.000Z" - assert items[0]["title"] == "Cybersecurity Resiliency is More Than Protection" + assert ( + items[0]["title"] == "Cybersecurity Resiliency & Stuff is More Than Protection" + ) + assert items[1]["title"] == ( + "From blockchain to virtual reality and beyond: Joshua Gayman\u2019s experience and Executive Certificate journey at MIT" + ) assert ( items[0]["url"] == "https://exec.mit.edu/s/blog-post/cybersecurity-resiliency-is-more-than-protection-20YU1000004rCaMMAU" @@ -52,6 +57,13 @@ def test_transform(): "Organizations & Leadership", "Trending Blog Posts", ] + assert items[3]["detail"]["authors"] == ['MIT Professor Fiona "OG" Murray'] + assert items[3]["summary"].startswith( + "

    Despite the many advances women have made in the workforce over" + ) + assert items[3]["content"].startswith( + "

    During Women\u2019s History Month, it is important" + ) def test_transform_no_posts(mocker): diff --git a/news_events/etl/utils.py b/news_events/etl/utils.py index dc833c45f5..395974060a 100644 --- a/news_events/etl/utils.py +++ b/news_events/etl/utils.py @@ -43,26 +43,6 @@ def tag_text(tag: Tag) -> str: return tag.text.strip() if tag and tag.text else None -def safe_html(tag: Tag) -> str: - """ - Get safe html from a BeautifulSoup tag, with no styles, classes, etc - - Args: - tag (Tag): The BeautifulSoup tag - - Returns: - str: The html as a string with certain elements/attributes removed - """ - if tag: - [element.decompose() for element in tag.findAll(["script", "style"])] - children = tag.find_all(recursive=True) - for element in [tag, *children]: - for attribute in ["style", "class"]: - del element[attribute] - return str(tag) - return None - - def stringify_time_struct(time_struct: struct_time) -> str: """ Transform a struct_time object into an ISO formatted date string diff --git a/news_events/etl/utils_test.py b/news_events/etl/utils_test.py index 70ece9f31f..a29c1a736f 100644 --- a/news_events/etl/utils_test.py +++ b/news_events/etl/utils_test.py @@ -36,16 +36,6 @@ def test_tag_text(mock_requests_get_html): ) -def test_safe_html(mock_requests_get_html): - """safe_html should return the html from a tag with no forbidden elements""" - soup = utils.get_soup("https://test.mit.edu/events") - initial_html = str(soup) - clean_html = utils.safe_html(soup) - for element in [" Fri, 15 Mar 2024 13:42:36 GMT 2024-03-15T13:58:19.238Z - Celebrating Women’s History Month with MIT women’s truncated]]> + Celebrating Women’s History Month with MIT women’s truncated]]> <![CDATA[Ten books from MIT faculty to expand your knowledge of teaching, learning, and technology]]> diff --git a/test_json/ol_events_output.json b/test_json/ol_events_output.json index e71791998e..b8331824dc 100644 --- a/test_json/ol_events_output.json +++ b/test_json/ol_events_output.json @@ -7,8 +7,8 @@ "alt": "students raising their hands", "description": "Students Raising Their Hands" }, - "summary": "The MicroMasters® program in Data, Economics, and Design of Policy (DEDP) equips learners with the practical skills and theoretical knowledge to tackle some of the most pressing challenges around the world. Through eight courses taught by MIT faculty, learners will gain a strong foundation in microeconomics, development economics, probability, and statistics, and engage with cutting-edge research in the field. The program is unique in its focus on running randomized evaluations to assess the effectiveness of social programs and its emphasis on hands-on skills in data analysis. All course content is available online for free. Learners who wish to earn certificates in the courses can pay a fee to take proctored exams. Learners who complete three core and two elective courses will earn a DEDP MicroMasters program credential in their respective track and become eligible to apply to the on-campus master’s in DEDP at MIT and other graduate programs at participating global pathway schools. Attend our 60-minute webinar to learn more and to have your questions answered live by the DEDP program team! Can't attend the live webinar? Register anyway! A recording will be sent to all registrants.Register >>", - "content": "The MicroMasters® program in Data, Economics, and Design of Policy (DEDP) equips learners with the practical skills and theoretical knowledge to tackle some of the most pressing challenges around the world. Through eight courses taught by MIT faculty, learners will gain a strong foundation in microeconomics, development economics, probability, and statistics, and engage with cutting-edge research in the field. The program is unique in its focus on running randomized evaluations to assess the effectiveness of social programs and its emphasis on hands-on skills in data analysis. All course content is available online for free. Learners who wish to earn certificates in the courses can pay a fee to take proctored exams. Learners who complete three core and two elective courses will earn a DEDP MicroMasters program credential in their respective track and become eligible to apply to the on-campus master’s in DEDP at MIT and other graduate programs at participating global pathway schools. Attend our 60-minute webinar to learn more and to have your questions answered live by the DEDP program team! Can't attend the live webinar? Register anyway! A recording will be sent to all registrants.Register >>", + "summary": "

    The MicroMasters® program in Data, Economics, and Design of Policy (DEDP) equips learners with the practical skills and theoretical knowledge to tackle some of the most pressing challenges around the world. 

    Through eight courses taught by MIT faculty, learners will gain a strong foundation in microeconomics, development economics, probability, and statistics, and engage with cutting-edge research in the field. The program is unique in its focus on running randomized evaluations to assess the effectiveness of social programs and its emphasis on hands-on skills in data analysis. 

    All course content is available online for free. Learners who wish to earn certificates in the courses can pay a fee to take proctored exams. Learners who complete three core and two elective courses will earn a DEDP MicroMasters program credential in their respective track and become eligible to apply to the on-campus master’s in DEDP at MIT and other graduate programs at participating global pathway schools. 

    Attend our 60-minute webinar to learn more and to have your questions answered live by the DEDP program team! 

    Can't attend the live webinar? Register anyway! A recording will be sent to all registrants.

    Register >>

    ", + "content": "

    The MicroMasters® program in Data, Economics, and Design of Policy (DEDP) equips learners with the practical skills and theoretical knowledge to tackle some of the most pressing challenges around the world. 

    Through eight courses taught by MIT faculty, learners will gain a strong foundation in microeconomics, development economics, probability, and statistics, and engage with cutting-edge research in the field. The program is unique in its focus on running randomized evaluations to assess the effectiveness of social programs and its emphasis on hands-on skills in data analysis. 

    All course content is available online for free. Learners who wish to earn certificates in the courses can pay a fee to take proctored exams. Learners who complete three core and two elective courses will earn a DEDP MicroMasters program credential in their respective track and become eligible to apply to the on-campus master’s in DEDP at MIT and other graduate programs at participating global pathway schools. 

    Attend our 60-minute webinar to learn more and to have your questions answered live by the DEDP program team! 

    Can't attend the live webinar? Register anyway! A recording will be sent to all registrants.

    Register >>

    ", "detail": { "location": ["Online"], "audience": ["Faculty", "MIT Community", "Public", "Students"], diff --git a/test_json/test_sloan_exec_news.json b/test_json/test_sloan_exec_news.json index 3357553daf..f20d45a2cb 100644 --- a/test_json/test_sloan_exec_news.json +++ b/test_json/test_sloan_exec_news.json @@ -40,7 +40,7 @@ }, "Article_Title": { "nodeType": "NameField", - "value": "Cybersecurity Resiliency is More Than Protection" + "value": "Cybersecurity Resiliency & Stuff is More Than Protection" }, "Original_Date": { "nodeType": "Date", @@ -63,7 +63,7 @@ "language": "en_US", "managedContentId": "20YU1000004rCaMMAU", "publishedDate": "2024-03-25T16:29:25.000Z", - "title": "Cybersecurity Resiliency is More Than Protection", + "title": "Cybersecurity Resiliency & Stuff is More Than Protection", "type": "Blog_Post", "typeLabel": "Blog Post", "unauthenticatedUrl": "/cms/delivery/v51.0/0DB6g0000004FHZGA2/contents/20YU1000004rCaMMAU?oid=00D6g000006yH4pEAE" @@ -205,7 +205,7 @@ "contentNodes": { "Quote_Author": { "nodeType": "Text", - "value": "MIT Professor Fiona Murray" + "value": "MIT Professor Fiona \u0022OG\u0022 Murray" }, "Featured_Image": { "altText": "pink fist punching through a glass ceiling", @@ -222,7 +222,7 @@ }, "First_Section_Content": { "nodeType": "RichText", - "value": "<p>During Women\u2019s History Month, it is important to celebrate the social, economic, cultural, and political achievements of women. This month also marks a call to action for accelerating gender parity. Many of the unique challenges women have traditionally faced when seeking to advance to senior levels appear to remain.</p>\n<p>In the quest to advance women, MIT Sloan Professor Fiona Murray and Senior Lecturer Elsbeth Johnson, co-faculty of the <a href="https://executive.mit.edu/course/womens-leadership-program/a054v00000rfdVZAAY.html" target="_blank">Women's Leadership Program</a>, advocate for a critical shift towards embracing power. Women often excel in influence and communication, yet may shy away from seeking power directly. This hesitation can hinder progress towards senior executive roles, despite exceptional expertise across fields like science, law, or finance. The challenge is to normalize the pursuit of power among women, encouraging them to step forward and claim their place at the decision-making table. Murray and Johnson offer six approaches to help aspiring women leaders address these challenges and break through the glass ceiling\u2014once and for all.</p>" + "value": "<p>During <a href='http://foo.com'>Women\u2019s History Month</a>, it is important to celebrate the social, economic, cultural, and political achievements of women. This month also marks a call to action for accelerating gender parity. Many of the unique challenges women have traditionally faced when seeking to advance to senior levels appear to remain.</p>\n<p>In the quest to advance women, MIT Sloan Professor Fiona Murray and Senior Lecturer Elsbeth Johnson, co-faculty of the <a href="https://executive.mit.edu/course/womens-leadership-program/a054v00000rfdVZAAY.html" target="_blank">Women's Leadership Program</a>, advocate for a critical shift towards embracing power. Women often excel in influence and communication, yet may shy away from seeking power directly. This hesitation can hinder progress towards senior executive roles, despite exceptional expertise across fields like science, law, or finance. The challenge is to normalize the pursuit of power among women, encouraging them to step forward and claim their place at the decision-making table. Murray and Johnson offer six approaches to help aspiring women leaders address these challenges and break through the glass ceiling\u2014once and for all.</p>" }, "Category": { "nodeType": "Text", @@ -242,7 +242,7 @@ }, "Summary": { "nodeType": "RichText", - "value": "<p>Despite the many advances women have made in the workforce over these past decades, many of the unique challenges women have traditionally faced when seeking to advance to senior levels appear to remain. Here are six tips from MIT's Fiona Murray and Elsbeth Johnson for claiming power and breaking through the glass ceiling once and for all.</p>" + "value": "<p>Despite the many advances women have made in the <a href='http://foo.com'>workforce</a> over these past decades, many of the unique challenges women have traditionally faced when seeking to advance to senior levels appear to remain. Here are six tips from MIT's Fiona Murray and Elsbeth Johnson for claiming power and breaking through the glass ceiling once and for all.</p>" }, "Featured_Quote": { "nodeType": "Text", From f822ba4ff1075d0b549bde3bde8894fb84129261 Mon Sep 17 00:00:00 2001 From: "renovate[bot]" <29139614+renovate[bot]@users.noreply.github.com> Date: Thu, 11 Jul 2024 13:29:03 -0400 Subject: [PATCH 08/11] Pin dependencies (#1225) Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> --- .github/workflows/publish-pages.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/publish-pages.yml b/.github/workflows/publish-pages.yml index 1a5961a74b..87e0c16a89 100644 --- a/.github/workflows/publish-pages.yml +++ b/.github/workflows/publish-pages.yml @@ -26,7 +26,7 @@ jobs: run: yarn workspace mit-open build-storybook - name: Upload artifact - uses: actions/upload-pages-artifact@v3 + uses: actions/upload-pages-artifact@56afc609e74202658d3ffba0e8f6dda462b719fa # v3 with: path: ./frontends/mit-open/storybook-static @@ -45,4 +45,4 @@ jobs: steps: - name: Deploy to GitHub Pages id: deployment - uses: actions/deploy-pages@v4 + uses: actions/deploy-pages@d6db90164ac5ed86f2b6aed7e0febac5b3c0c03e # v4 From cb21d9aa0a87ab4d04b5b0a2f876bdc7c33f99ef Mon Sep 17 00:00:00 2001 From: Anastasia Beglova Date: Thu, 11 Jul 2024 13:44:45 -0400 Subject: [PATCH 09/11] reindexing fixes (#1247) --- learning_resources_search/indexing_api.py | 42 ++++++-- .../indexing_api_test.py | 46 ++++++--- .../management/commands/recreate_index.py | 36 +++++-- learning_resources_search/tasks.py | 25 ++++- learning_resources_search/tasks_test.py | 99 ++++++++++++++++++- 5 files changed, 210 insertions(+), 38 deletions(-) diff --git a/learning_resources_search/indexing_api.py b/learning_resources_search/indexing_api.py index a51dcc8c0c..6b6c3dd21c 100644 --- a/learning_resources_search/indexing_api.py +++ b/learning_resources_search/indexing_api.py @@ -514,20 +514,46 @@ def switch_indices(backing_index, object_type): ) -def delete_orphaned_indices(): +def delete_orphaned_indexes(obj_types, delete_reindexing_tags): """ - Delete any indices without aliases and any reindexing aliases + Delete any indices without aliases """ conn = get_conn() indices = conn.indices.get_alias(index="*") for index in indices: aliases = indices[index]["aliases"] keys = list(aliases) + + if delete_reindexing_tags: + for alias in aliases: + if "reindexing" in alias: + log.info("Deleting alias %s for index %s", alias, index) + conn.indices.delete_alias(name=alias, index=index) + keys.remove(alias) + + if not keys and not index.startswith("."): + for object_type in obj_types: + if object_type in index: + log.info("Deleting orphaned index %s", index) + conn.indices.delete(index) + break + + +def get_existing_reindexing_indexes(obj_types): + """ + Check for existing indexes with reindexing tag + """ + conn = get_conn() + reindexing_indexes = [] + indices = conn.indices.get_alias(index="*") + for index in indices: + aliases = indices[index]["aliases"] + for alias in aliases: if "reindexing" in alias: - log.info("Deleting alias %s for index %s", alias, index) - conn.indices.delete_alias(name=alias, index=index) - keys.remove(alias) - if not keys: - log.info("Deleting index %s", index) - conn.indices.delete(index) + for object_type in obj_types: + if object_type in index: + reindexing_indexes.append(index) + break + + return reindexing_indexes diff --git a/learning_resources_search/indexing_api_test.py b/learning_resources_search/indexing_api_test.py index 4befc98b58..50f9f31491 100644 --- a/learning_resources_search/indexing_api_test.py +++ b/learning_resources_search/indexing_api_test.py @@ -32,7 +32,7 @@ deindex_learning_resources, deindex_percolators, deindex_run_content_files, - delete_orphaned_indices, + delete_orphaned_indexes, get_reindexing_alias_name, index_content_files, index_course_content_files, @@ -349,7 +349,8 @@ def test_index_items_size_limits(settings, mocker, max_size, chunks, exceeds_siz assert mock_log.call_count == (10 if exceeds_size else 0) -def test_delete_orphaned_indices(mocker, mocked_es): +@pytest.mark.parametrize("delete_reindexing_tags", [True, False]) +def test_delete_orphaned_indexes(mocker, mocked_es, delete_reindexing_tags): """ Delete any indices without aliases and any reindexing aliases """ @@ -369,6 +370,12 @@ def test_delete_orphaned_indices(mocker, mocked_es): "some_other_alias": {}, } }, + "discussions_local_podcast_1e61d15ff35b8c9b4f6884bba05484cb": { + "aliases": { + "discussions_local_program_reindexing": {}, + "some_other_alias": {}, + } + }, "discussions_local_program_5484cbb8c9b1e61d15ff354f6884bba0": {"aliases": {}}, "discussions_local_course_15ff354d6884bba05484cbb8c9b1e61d": { "aliases": { @@ -380,23 +387,32 @@ def test_delete_orphaned_indices(mocker, mocked_es): mocked_es.conn.indices = mocker.Mock( delete_alias=mocker.Mock(), get_alias=mocker.Mock(return_value=mock_aliases) ) - delete_orphaned_indices() + delete_orphaned_indexes(["program"], delete_reindexing_tags=delete_reindexing_tags) mocked_es.conn.indices.get_alias.assert_called_once_with(index="*") - mocked_es.conn.indices.delete_alias.assert_any_call( - name="discussions_local_program_reindexing", - index="discussions_local_program_b8c9b1e61d15ff354f6884bba05484cb", - ) - mocked_es.conn.indices.delete_alias.assert_any_call( - name="discussions_local_program_reindexing", - index="discussions_local_program_1e61d15ff35b8c9b4f6884bba05484cb", - ) + + if delete_reindexing_tags: + mocked_es.conn.indices.delete_alias.assert_any_call( + name="discussions_local_program_reindexing", + index="discussions_local_program_b8c9b1e61d15ff354f6884bba05484cb", + ) + mocked_es.conn.indices.delete_alias.assert_any_call( + name="discussions_local_program_reindexing", + index="discussions_local_program_1e61d15ff35b8c9b4f6884bba05484cb", + ) + else: + mocked_es.conn.indices.delete_alias.assert_not_called() + mocked_es.conn.indices.delete.assert_any_call( "discussions_local_program_5484cbb8c9b1e61d15ff354f6884bba0" ) - mocked_es.conn.indices.delete.assert_any_call( - "discussions_local_program_b8c9b1e61d15ff354f6884bba05484cb" - ) - assert mocked_es.conn.indices.delete.call_count == 2 + + if delete_reindexing_tags: + mocked_es.conn.indices.delete.assert_any_call( + "discussions_local_program_b8c9b1e61d15ff354f6884bba05484cb" + ) + assert mocked_es.conn.indices.delete.call_count == 2 + else: + assert mocked_es.conn.indices.delete.call_count == 1 def test_bulk_content_file_deindex_on_course_deletion(mocker): diff --git a/learning_resources_search/management/commands/recreate_index.py b/learning_resources_search/management/commands/recreate_index.py index 6969f9c7f8..6d47b10b63 100644 --- a/learning_resources_search/management/commands/recreate_index.py +++ b/learning_resources_search/management/commands/recreate_index.py @@ -3,6 +3,7 @@ from django.core.management.base import BaseCommand, CommandError from learning_resources_search.constants import ALL_INDEX_TYPES +from learning_resources_search.indexing_api import get_existing_reindexing_indexes from learning_resources_search.tasks import start_recreate_index from main.utils import now_in_utc @@ -13,6 +14,13 @@ class Command(BaseCommand): help = "Recreate opensearch index" def add_arguments(self, parser): + parser.add_argument( + "--remove_existing_reindexing_tags", + dest="remove_existing_reindexing_tags", + action="store_true", + help="Overwrite any existing reindexing tags and remove those indexes", + ) + parser.add_argument( "--all", dest="all", action="store_true", help="Recreate all indexes" ) @@ -28,11 +36,9 @@ def add_arguments(self, parser): def handle(self, *args, **options): # noqa: ARG002 """Index all LEARNING_RESOURCE_TYPES""" + remove_existing_reindexing_tags = options["remove_existing_reindexing_tags"] if options["all"]: - task = start_recreate_index.delay(list(ALL_INDEX_TYPES)) - self.stdout.write( - f"Started celery task {task} to index content for all indexes" - ) + indexes_to_update = list(ALL_INDEX_TYPES) else: indexes_to_update = list( filter(lambda object_type: options[object_type], ALL_INDEX_TYPES) @@ -44,12 +50,24 @@ def handle(self, *args, **options): # noqa: ARG002 for object_type in sorted(ALL_INDEX_TYPES): self.stdout.write(f" --{object_type}s") return - - task = start_recreate_index.delay(indexes_to_update) - self.stdout.write( - f"Started celery task {task} to index content for the following" - f" indexes: {indexes_to_update}" + if not remove_existing_reindexing_tags: + existing_reindexing_indexes = get_existing_reindexing_indexes( + indexes_to_update ) + if existing_reindexing_indexes: + self.stdout.write( + f"Reindexing in progress. Reindexing indexes already exist:" + f" {', '.join(existing_reindexing_indexes)}" + ) + return + + task = start_recreate_index.delay( + indexes_to_update, remove_existing_reindexing_tags + ) + self.stdout.write( + f"Started celery task {task} to index content for the following" + f" indexes: {indexes_to_update}" + ) self.stdout.write("Waiting on task...") start = now_in_utc() diff --git a/learning_resources_search/tasks.py b/learning_resources_search/tasks.py index 8d941b4cb1..1d94d7b935 100644 --- a/learning_resources_search/tasks.py +++ b/learning_resources_search/tasks.py @@ -458,11 +458,26 @@ def wrap_retry_exception(*exception_classes): @app.task(bind=True) -def start_recreate_index(self, indexes): +def start_recreate_index(self, indexes, remove_existing_reindexing_tags): """ Wipe and recreate index and mapping, and index all items. """ try: + if not remove_existing_reindexing_tags: + existing_reindexing_indexes = api.get_existing_reindexing_indexes(indexes) + + if existing_reindexing_indexes: + error = ( + f"Reindexing in progress. Reindexing indexes already exist: " + f"{', '.join(existing_reindexing_indexes)}" + ) + log.exception(error) + return error + + api.delete_orphaned_indexes( + indexes, delete_reindexing_tags=remove_existing_reindexing_tags + ) + new_backing_indices = { obj_type: api.create_backing_index(obj_type) for obj_type in indexes } @@ -768,7 +783,9 @@ def get_update_learning_resource_tasks(resource_type): ] -@app.task(autoretry_for=(RetryError,), retry_backoff=True, rate_limit="600/m") +@app.task( + acks_late=True, autoretry_for=(RetryError,), retry_backoff=True, rate_limit="600/m" +) def finish_recreate_index(results, backing_indices): """ Swap reindex backing index with default backing index @@ -780,7 +797,9 @@ def finish_recreate_index(results, backing_indices): errors = merge_strings(results) if errors: try: - api.delete_orphaned_indices() + api.delete_orphaned_indexes( + list(backing_indices.keys()), delete_reindexing_tags=True + ) except RequestError as ex: raise RetryError(str(ex)) from ex msg = f"Errors occurred during recreate_index: {errors}" diff --git a/learning_resources_search/tasks_test.py b/learning_resources_search/tasks_test.py index 701b650a13..a65ac7d0e9 100644 --- a/learning_resources_search/tasks_test.py +++ b/learning_resources_search/tasks_test.py @@ -165,15 +165,27 @@ def test_start_recreate_index(mocker, mocked_celery, user, indexes): autospec=True, return_value=backing_index, ) + mocker.patch( + "learning_resources_search.indexing_api.get_existing_reindexing_indexes", + autospec=True, + return_value=[], + ) + delete_orphaned_indexes_mock = mocker.patch( + "learning_resources_search.indexing_api.delete_orphaned_indexes", autospec=True + ) finish_recreate_index_mock = mocker.patch( "learning_resources_search.tasks.finish_recreate_index", autospec=True ) with pytest.raises(mocked_celery.replace_exception_class): - start_recreate_index.delay(indexes) + start_recreate_index.delay(indexes, remove_existing_reindexing_tags=False) finish_recreate_index_dict = {} + delete_orphaned_indexes_mock.assert_called_once_with( + indexes, delete_reindexing_tags=False + ) + for doctype in LEARNING_RESOURCE_TYPES: if doctype in indexes: finish_recreate_index_dict[doctype] = backing_index @@ -240,6 +252,87 @@ def test_start_recreate_index(mocker, mocked_celery, user, indexes): assert mocked_celery.replace.call_args[0][1] == mocked_celery.chain.return_value +@pytest.mark.parametrize( + "remove_existing_reindexing_tags", + [True, False], +) +def test_start_recreate_index_existing_reindexing_index( + mocker, mocked_celery, user, remove_existing_reindexing_tags +): + settings.OPENSEARCH_INDEXING_CHUNK_SIZE = 2 + settings.OPENSEARCH_DOCUMENT_INDEXING_CHUNK_SIZE = 2 + indexes = ["program"] + + programs = sorted( + ProgramFactory.create_batch(4), + key=lambda program: program.learning_resource_id, + ) + + index_learning_resources_mock = mocker.patch( + "learning_resources_search.tasks.index_learning_resources", autospec=True + ) + + backing_index = "backing" + mocker.patch( + "learning_resources_search.indexing_api.create_backing_index", + autospec=True, + return_value=backing_index, + ) + delete_orphaned_indexes_mock = mocker.patch( + "learning_resources_search.indexing_api.delete_orphaned_indexes", autospec=True + ) + finish_recreate_index_mock = mocker.patch( + "learning_resources_search.tasks.finish_recreate_index", autospec=True + ) + + mocker.patch( + "learning_resources_search.indexing_api.get_existing_reindexing_indexes", + autospec=True, + return_value=["another_reindexing_index"], + ) + + if remove_existing_reindexing_tags: + with pytest.raises(mocked_celery.replace_exception_class): + start_recreate_index.delay( + indexes, remove_existing_reindexing_tags=remove_existing_reindexing_tags + ) + else: + start_recreate_index.delay( + indexes, remove_existing_reindexing_tags=remove_existing_reindexing_tags + ) + + finish_recreate_index_dict = {"program": "backing"} + + if remove_existing_reindexing_tags: + delete_orphaned_indexes_mock.assert_called_once_with( + indexes, delete_reindexing_tags=True + ) + + finish_recreate_index_mock.s.assert_called_once_with(finish_recreate_index_dict) + assert mocked_celery.group.call_count == 1 + + # Celery's 'group' function takes a generator as an argument. In order to make assertions about the items + # in that generator, 'list' is being called to force iteration through all of those items. + list(mocked_celery.group.call_args[0][0]) + assert index_learning_resources_mock.si.call_count == 2 + index_learning_resources_mock.si.assert_any_call( + [programs[0].learning_resource_id, programs[1].learning_resource_id], + PROGRAM_TYPE, + index_types=IndexestoUpdate.reindexing_index.value, + ) + index_learning_resources_mock.si.assert_any_call( + [programs[2].learning_resource_id, programs[3].learning_resource_id], + PROGRAM_TYPE, + index_types=IndexestoUpdate.reindexing_index.value, + ) + + assert mocked_celery.replace.call_count == 1 + assert mocked_celery.replace.call_args[0][1] == mocked_celery.chain.return_value + else: + assert index_learning_resources_mock.si.call_count == 0 + assert mocked_celery.replace.call_count == 0 + + @pytest.mark.parametrize("with_error", [True, False]) def test_finish_recreate_index(mocker, with_error): """ @@ -251,7 +344,7 @@ def test_finish_recreate_index(mocker, with_error): "learning_resources_search.indexing_api.switch_indices", autospec=True ) mock_delete_orphans = mocker.patch( - "learning_resources_search.indexing_api.delete_orphaned_indices" + "learning_resources_search.indexing_api.delete_orphaned_indexes" ) if with_error: @@ -280,7 +373,7 @@ def test_finish_recreate_index_retry_exceptions(mocker, with_error): side_effect=[mock_error, None], ) mock_delete_orphans = mocker.patch( - "learning_resources_search.indexing_api.delete_orphaned_indices", + "learning_resources_search.indexing_api.delete_orphaned_indexes", side_effect=[mock_error, None], ) From a324ca968e7543ad1c0aff3ff82f02e3be5cf32f Mon Sep 17 00:00:00 2001 From: Chris Chudzicki Date: Thu, 11 Jul 2024 14:41:02 -0400 Subject: [PATCH 10/11] change xpro ETL dict key back (#1252) * change xpro ETL dict key back * change xpro mock api data back --- learning_resources/etl/xpro.py | 2 +- test_json/xpro_courses.json | 14 ++++----- test_json/xpro_programs.json | 56 ++++++++++++++++++++++++---------- 3 files changed, 48 insertions(+), 24 deletions(-) diff --git a/learning_resources/etl/xpro.py b/learning_resources/etl/xpro.py index 24fd4de42d..d4d5b95827 100644 --- a/learning_resources/etl/xpro.py +++ b/learning_resources/etl/xpro.py @@ -33,7 +33,7 @@ "Simplilearn": PlatformType.simplilearn.name, "Susskind": PlatformType.susskind.name, "WHU": PlatformType.whu.name, - "MIT xPRO": PlatformType.xpro.name, + "xPRO": PlatformType.xpro.name, } diff --git a/test_json/xpro_courses.json b/test_json/xpro_courses.json index 87f6f924d2..8d55cf2924 100644 --- a/test_json/xpro_courses.json +++ b/test_json/xpro_courses.json @@ -8,7 +8,7 @@ "readable_id": "course-v1:xPRO+SysEngxB1", "courseruns": [], "next_run_id": null, - "platform": "MIT xPRO", + "platform": "xPRO", "topics": [{ "name": "Business:Leadership & Organizations" }], "format": "Online" }, @@ -18,7 +18,7 @@ "description": "

    Course 3 of 4 that comprises the Architecture and Systems Engineering Professional Certificate Program. This course may be taken individually, without enrolling in the professional certificate program.

    ", "thumbnail_url": "/static/images/mit-dome.png", "readable_id": "course-v1:xPRO+SysEngxB3", - "platform": "MIT xPRO", + "platform": "xPRO", "courseruns": [ { "title": "SEED Model-Based Systems Engineering: Documentation and Analysis - Fall 2020", @@ -46,7 +46,7 @@ "thumbnail_url": "/static/images/mit-dome.png", "readable_id": "course-v1:xPRO+SysEngxB2", "url": "https://xpro.mit.edu/courses/course-v1:xPRO+SysEngxB2", - "platform": "MIT xPRO", + "platform": "xPRO", "courseruns": [ { "title": "SEED Models in Engineering - Spring 2020", @@ -76,7 +76,7 @@ "courseruns": [], "next_run_id": null, "topics": [{ "name": "Business:Leadership & Organizations" }], - "platform": "MIT xPRO", + "platform": "xPRO", "format": "Online" }, { @@ -85,7 +85,7 @@ "description": "

    An introductory course to Digital Learning

    ", "thumbnail_url": "/static/images/mit-dome.png", "readable_id": "course-v1:xPRO+DgtlLearn1", - "platform": "MIT xPRO", + "platform": "xPRO", "format": "Online", "courseruns": [ { @@ -138,7 +138,7 @@ "next_run_id": null, "topics": [{ "name": "Business:Leadership & Organizations" }], "format": "Online", - "platform": "MIT xPRO" + "platform": "xPRO" }, { "id": 32, @@ -150,6 +150,6 @@ "next_run_id": null, "topics": [{ "name": "Business:Leadership & Organizations" }], "format": "Online", - "platform": "MIT xPRO" + "platform": "xPRO" } ] diff --git a/test_json/xpro_programs.json b/test_json/xpro_programs.json index efbbdcc62e..8288ee998f 100644 --- a/test_json/xpro_programs.json +++ b/test_json/xpro_programs.json @@ -11,11 +11,15 @@ "end_date": "2019-12-01T00:00:00Z", "enrollment_start": "2019-08-01T00:00:00Z", "topics": [ - { "name": "Business:Leadership & Organizations" }, - { "name": "Planning" } + { + "name": "Business:Leadership & Organizations" + }, + { + "name": "Planning" + } ], "format": "Online", - "platform": "MIT xPRO", + "platform": "xPRO", "courses": [ { "id": 18, @@ -25,8 +29,12 @@ "readable_id": "course-v1:xPRO+SysEngxB1", "courseruns": [], "next_run_id": null, - "topics": [{ "name": "Business:Leadership & Organizations" }], - "platform": "MIT xPRO", + "topics": [ + { + "name": "Business:Leadership & Organizations" + } + ], + "platform": "xPRO", "format": "Online" }, { @@ -36,7 +44,7 @@ "thumbnail_url": "/static/images/mit-dome.png", "readable_id": "course-v1:xPRO+SysEngxB3", "topics": [], - "platform": "MIT xPRO", + "platform": "xPRO", "format": "Online", "courseruns": [ { @@ -62,8 +70,12 @@ "description": "

    Course 2 of 4 that comprises the Architecture and Systems Engineering Professional Certificate Program. This course may be taken individually, without enrolling in the professional certificate program.

    ", "thumbnail_url": "/static/images/mit-dome.png", "readable_id": "course-v1:xPRO+SysEngxB2", - "topics": [{ "name": "Business:Leadership & Organizations" }], - "platform": "MIT xPRO", + "topics": [ + { + "name": "Business:Leadership & Organizations" + } + ], + "platform": "xPRO", "format": "Online", "courseruns": [ { @@ -92,7 +104,7 @@ "courseruns": [], "next_run_id": null, "topics": [], - "platform": "MIT xPRO" + "platform": "xPRO" } ] }, @@ -107,9 +119,13 @@ "start_date": "2019-09-10T00:00:00Z", "end_date": "2019-12-10T00:00:00Z", "enrollment_start": null, - "topics": [{ "name": "Business:Leadership & Organizations" }], + "topics": [ + { + "name": "Business:Leadership & Organizations" + } + ], "format": "In person", - "platform": "MIT xPRO", + "platform": "xPRO", "courses": [ { "id": 30, @@ -117,8 +133,12 @@ "description": "

    An introductory course to Digital Learning

    ", "thumbnail_url": "/static/images/mit-dome.png", "readable_id": "course-v1:xPRO+DgtlLearn1", - "topics": [{ "name": "Business:Leadership & Organizations" }], - "platform": "MIT xPRO", + "topics": [ + { + "name": "Business:Leadership & Organizations" + } + ], + "platform": "xPRO", "courseruns": [ { "title": "SEED Digital Learning 100 - August 2020", @@ -167,8 +187,12 @@ "readable_id": "course-v1:xPRO+DgtlLearn2", "courseruns": [], "next_run_id": null, - "topics": [{ "name": "Business:Leadership & Organizations" }], - "platform": "MIT xPRO" + "topics": [ + { + "name": "Business:Leadership & Organizations" + } + ], + "platform": "xPRO" }, { "id": 32, @@ -179,7 +203,7 @@ "courseruns": [], "next_run_id": null, "topics": [], - "platform": "MIT xPRO" + "platform": "xPRO" } ] } From 29c4975a2008853b9b343068be40d51dc62e9d28 Mon Sep 17 00:00:00 2001 From: Doof Date: Thu, 11 Jul 2024 18:41:40 +0000 Subject: [PATCH 11/11] Release 0.13.19 --- RELEASE.rst | 14 ++++++++++++++ main/settings.py | 2 +- 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/RELEASE.rst b/RELEASE.rst index 199cd70966..dabd690f3c 100644 --- a/RELEASE.rst +++ b/RELEASE.rst @@ -1,6 +1,20 @@ Release Notes ============= +Version 0.13.19 +--------------- + +- change xpro ETL dict key back (#1252) +- reindexing fixes (#1247) +- Pin dependencies (#1225) +- Plain text news/events titles/authors; standardize html cleanup (#1248) +- Condensed list card components for user lists (#1251) +- Change readable_id values for podcasts and episodes (#1232) +- adjust / refactor channel detail header (#1234) +- use main not "$default-branch" (#1249) +- Update dependency ruff to v0.5.1 (#1241) +- Update dependency Django to v4.2.14 (#1240) + Version 0.13.18 (Released July 10, 2024) --------------- diff --git a/main/settings.py b/main/settings.py index 067b6aa887..871f2920dc 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.18" +VERSION = "0.13.19" log = logging.getLogger()