From 03e03f60d32a0b376aaaa523109a75f7d60c0114 Mon Sep 17 00:00:00 2001 From: Muhammad Anas <88967643+Anas12091101@users.noreply.github.com> Date: Wed, 15 Apr 2026 19:38:21 +0500 Subject: [PATCH 1/5] feat: enable course access to staff user before the start date (#3182) * feat: enable course access to staff user before the start date * fix: issues * fix: test --- .../CoursewareDisplay/DashboardCard.test.tsx | 40 +++++++++++++++++-- .../CoursewareDisplay/DashboardCard.tsx | 16 +++++++- .../CoursewareDisplay/ModuleCard.tsx | 12 +++++- 3 files changed, 61 insertions(+), 7 deletions(-) diff --git a/frontends/main/src/app-pages/DashboardPage/CoursewareDisplay/DashboardCard.test.tsx b/frontends/main/src/app-pages/DashboardPage/CoursewareDisplay/DashboardCard.test.tsx index e64ca1167c..25a73fcc08 100644 --- a/frontends/main/src/app-pages/DashboardPage/CoursewareDisplay/DashboardCard.test.tsx +++ b/frontends/main/src/app-pages/DashboardPage/CoursewareDisplay/DashboardCard.test.tsx @@ -74,9 +74,13 @@ const futureDashboardCourse: typeof mitxOnlineCourse = (...overrides) => { const mitxUser = mitxonline.factories.user.user -const setupUserApis = () => { - const mitxUser = mitxonline.factories.user.user() - setMockResponse.get(mitxonline.urls.userMe.get(), mitxUser) +const setupUserApis = (overrides?: Parameters[0]) => { + const userData = mitxonline.factories.user.user({ + is_staff: false, + ...overrides, + }) + setMockResponse.get(mitxonline.urls.userMe.get(), userData) + return userData } describe.each([ @@ -246,6 +250,36 @@ describe.each([ }, ) + test("Courseware CTA is a navigable link for staff even when course has not started", async () => { + setupUserApis({ is_staff: true }) + const coursewareUrl = faker.internet.url() + const course = futureDashboardCourse() + const enrollment = mitxonline.factories.enrollment.courseEnrollment({ + enrollment_mode: EnrollmentMode.Audit, + grades: [], + certificate: null, + run: { + ...course.courseruns[0], + course: course, + courseware_url: coursewareUrl, + }, + }) + renderWithProviders( + , + ) + const card = getCard() + const coursewareCTA = await within(card).findByRole("link", { + name: "Continue", + }) + expect(coursewareCTA).toBeEnabled() + expect(coursewareCTA).toHaveAttribute("href", coursewareUrl) + }) + test("Courseware CTA is disabled when no enrollable runs exist", () => { setupUserApis() const course = mitxOnlineCourse({ diff --git a/frontends/main/src/app-pages/DashboardPage/CoursewareDisplay/DashboardCard.tsx b/frontends/main/src/app-pages/DashboardPage/CoursewareDisplay/DashboardCard.tsx index 5a6a8a1c22..84ca2d3d0b 100644 --- a/frontends/main/src/app-pages/DashboardPage/CoursewareDisplay/DashboardCard.tsx +++ b/frontends/main/src/app-pages/DashboardPage/CoursewareDisplay/DashboardCard.tsx @@ -389,6 +389,7 @@ const useEnrollmentHandler = () => { createEnrollment.isPending || createVerifiedProgramEnrollment.isPending || replaceBasketItem.isPending, + mitxOnlineUser: mitxOnlineUser.data, } } @@ -402,6 +403,7 @@ type CoursewareButtonProps = { noun: string isProgram?: boolean isPending?: boolean + isStaff?: boolean "data-testid"?: string onClick?: React.MouseEventHandler } @@ -445,6 +447,7 @@ const CoursewareButton = styled( isProgram, onClick, isPending, + isStaff, ...others }: CoursewareButtonProps) => { const coursewareText = getCoursewareTextAndIcon({ @@ -457,7 +460,12 @@ const CoursewareButton = styled( const hasEnrolled = enrollmentStatus !== EnrollmentStatus.NotEnrolled // Programs or enrolled courses with started runs: show link - if ((isProgram || hasEnrolled) && (hasStarted || !startDate) && href) { + // Staff can access courseware even before the course has started + if ( + (isProgram || hasEnrolled) && + (hasStarted || !startDate || isStaff) && + href + ) { return ( = ({ onUpgradeError, }) => { const enrollment = useEnrollmentHandler() + const mitxOnlineUser = enrollment.mitxOnlineUser const useProductPages = useFeatureFlagEnabled( FeatureFlags.MitxOnlineProductPages, ) @@ -835,6 +846,7 @@ const DashboardCard: React.FC = ({ isProgram={false} disabled={disableEnrollment} isPending={enrollment.isPending} + isStaff={mitxOnlineUser?.is_staff} onClick={coursewareButtonClick} /> diff --git a/frontends/main/src/app-pages/DashboardPage/CoursewareDisplay/ModuleCard.tsx b/frontends/main/src/app-pages/DashboardPage/CoursewareDisplay/ModuleCard.tsx index 0764c0221c..58018e2b9d 100644 --- a/frontends/main/src/app-pages/DashboardPage/CoursewareDisplay/ModuleCard.tsx +++ b/frontends/main/src/app-pages/DashboardPage/CoursewareDisplay/ModuleCard.tsx @@ -366,6 +366,7 @@ const useEnrollmentHandler = () => { createEnrollment.isPending || createVerifiedProgramEnrollment.isPending || replaceBasketItem.isPending, + mitxOnlineUser: mitxOnlineUser.data, } } @@ -376,6 +377,7 @@ type CoursewareButtonProps = { href?: string | null disabled?: boolean className?: string + isStaff?: boolean "data-testid"?: string onClick?: React.MouseEventHandler } @@ -413,6 +415,7 @@ const CoursewareButton = styled( disabled, className, onClick, + isStaff, ...others }: CoursewareButtonProps) => { const coursewareText = getCoursewareButtonStyle({ @@ -422,7 +425,8 @@ const CoursewareButton = styled( const hasStarted = startDate && isInPast(startDate) const hasEnrolled = enrollmentStatus !== EnrollmentStatus.NotEnrolled - if (hasEnrolled && (hasStarted || !startDate) && href) { + // Staff can access courseware even before the course has started + if (hasEnrolled && (hasStarted || !startDate || isStaff) && href) { return ( = ({ onUpgradeError, }) => { const enrollment = useEnrollmentHandler() + const mitxOnlineUser = enrollment.mitxOnlineUser const title = getTitle(resource) const enrollmentStatus = getDashboardEnrollmentStatus(resource) @@ -838,6 +845,7 @@ const DashboardCourseCard: React.FC = ({ href={buttonHref ?? coursewareUrl} endDate={courseRun?.end_date ?? enrollmentRun?.end_date} disabled={disableEnrollment} + isStaff={mitxOnlineUser?.is_staff} onClick={coursewareButtonClick} /> From c182068b815746fc50fdb96905a63beabc5573f9 Mon Sep 17 00:00:00 2001 From: Matt Bertrand Date: Wed, 15 Apr 2026 10:42:07 -0700 Subject: [PATCH 2/5] Deindex unpublished test_mode resources, not contentfiles (#3192) --- learning_resources/etl/loaders.py | 6 +--- learning_resources/etl/loaders_test.py | 8 +++--- learning_resources_search/plugins.py | 4 ++- learning_resources_search/plugins_test.py | 35 +++++++++++++++++++---- 4 files changed, 38 insertions(+), 15 deletions(-) diff --git a/learning_resources/etl/loaders.py b/learning_resources/etl/loaders.py index a64eb7c92e..d1c6fe01a5 100644 --- a/learning_resources/etl/loaders.py +++ b/learning_resources/etl/loaders.py @@ -80,11 +80,7 @@ def update_index(learning_resource, newly_created): learning resource (LearningResource): a learning resource newly_created (bool): whether the learning resource has just been created """ - if ( - not newly_created - and not learning_resource.published - and not learning_resource.test_mode - ): + if not newly_created and not learning_resource.published: resource_unpublished_actions(learning_resource) elif learning_resource.published: resource_upserted_actions( diff --git a/learning_resources/etl/loaders_test.py b/learning_resources/etl/loaders_test.py index bdcd11ea55..8649bd7c25 100644 --- a/learning_resources/etl/loaders_test.py +++ b/learning_resources/etl/loaders_test.py @@ -185,17 +185,17 @@ def test_update_index_test_mode_behavior( test_mode, newly_created, ): - """Test update_index does not remove test_mode content files from index""" + """Test update_index unpublishes existing unpublished resources""" resource_unpublished_actions = mocker.patch( "learning_resources.etl.loaders.resource_unpublished_actions" ) lr = LearningResourceFactory.create(published=published, test_mode=test_mode) loaders.update_index(lr, newly_created) - if test_mode: - resource_unpublished_actions.assert_not_called() - elif not published and not newly_created: + if not published and not newly_created: resource_unpublished_actions.assert_called_once() + else: + resource_unpublished_actions.assert_not_called() @pytest.fixture diff --git a/learning_resources_search/plugins.py b/learning_resources_search/plugins.py index 1882622475..34e245592c 100644 --- a/learning_resources_search/plugins.py +++ b/learning_resources_search/plugins.py @@ -104,7 +104,7 @@ def resource_unpublished(self, resource): ) try_with_retry_as_task(chain(*unpublished_tasks)) - if resource.resource_type == COURSE_TYPE: + if resource.resource_type == COURSE_TYPE and not resource.test_mode: for run in resource.runs.all(): self.resource_run_unpublished(run) @@ -159,6 +159,8 @@ def resource_before_delete(self, resource): """ Remove a resource from the search index and then delete the object """ + # Ensure test mode is false so the resource is removed from the search index + resource.test_mode = False self.resource_unpublished(resource) @hookimpl diff --git a/learning_resources_search/plugins_test.py b/learning_resources_search/plugins_test.py index b4ff44c541..517614a596 100644 --- a/learning_resources_search/plugins_test.py +++ b/learning_resources_search/plugins_test.py @@ -89,11 +89,14 @@ def test_search_index_plugin_resource_upserted( @pytest.mark.django_db @pytest.mark.parametrize("resource_type", [COURSE_TYPE, PROGRAM_TYPE]) @pytest.mark.parametrize("has_content_files", [True, False]) +@pytest.mark.parametrize("test_mode", [True, False]) def test_search_index_plugin_resource_unpublished( - mocker, mock_search_index_helpers, resource_type, has_content_files + mocker, mock_search_index_helpers, resource_type, has_content_files, test_mode ): """The plugin function should remove a resource from the search index""" - resource = LearningResourceFactory.create(resource_type=resource_type) + resource = LearningResourceFactory.create( + resource_type=resource_type, test_mode=test_mode + ) if resource_type == COURSE_TYPE and has_content_files: for run in resource.runs.all(): ContentFileFactory.create(run=run) @@ -104,7 +107,7 @@ def test_search_index_plugin_resource_unpublished( mock_search_index_helpers.mock_remove_learning_resource_immutable_signature.assert_called_once_with( resource.id, resource.resource_type ) - if resource_type == COURSE_TYPE and has_content_files: + if resource_type == COURSE_TYPE and has_content_files and not test_mode: assert unpublish_run_mock.call_count == resource.runs.count() for run in resource.runs.all(): unpublish_run_mock.assert_any_call(run.id, unpublished_only=False) @@ -114,17 +117,39 @@ def test_search_index_plugin_resource_unpublished( @pytest.mark.django_db @pytest.mark.parametrize("resource_type", [COURSE_TYPE, PROGRAM_TYPE]) +@pytest.mark.parametrize("test_mode", [True, False]) def test_search_index_plugin_resource_before_delete( - mock_search_index_helpers, resource_type + mock_search_index_helpers, resource_type, test_mode ): """The plugin function should remove a resource from the search index then delete the resource""" - resource = LearningResourceFactory.create(resource_type=resource_type) + resource = LearningResourceFactory.create( + resource_type=resource_type, + test_mode=test_mode, + ) + if resource_type == COURSE_TYPE: + for run in resource.runs.all(): + ContentFileFactory.create(run=run) + resource_id = resource.id SearchIndexPlugin().resource_before_delete(resource) mock_search_index_helpers.mock_remove_learning_resource_immutable_signature.assert_called_once_with( resource_id, resource.resource_type ) + assert resource.test_mode is False + + if resource_type == COURSE_TYPE: + assert ( + mock_search_index_helpers.mock_remove_contentfiles_immutable_signature.call_count + == resource.runs.count() + ) + for run in resource.runs.all(): + mock_search_index_helpers.mock_remove_contentfiles_immutable_signature.assert_any_call( + run.id, + unpublished_only=False, + ) + else: + mock_search_index_helpers.mock_remove_contentfiles_immutable_signature.assert_not_called() @pytest.mark.django_db From 9b381d3d528136dd5aee7f16606b4b210c608298 Mon Sep 17 00:00:00 2001 From: Carey P Gumaer Date: Wed, 15 Apr 2026 15:04:19 -0400 Subject: [PATCH 3/5] hide stay updated button on free products (#3201) * hide stay updated button on free products * disable query if button is to be disabled * handle test form id better * missed committing helper file * actually this wasn't supposed to be here * make sure enabled is always a boolean --- .../ProductPages/CoursePage.test.tsx | 93 +++++++++++++++++++ .../src/app-pages/ProductPages/CoursePage.tsx | 11 +++ .../ProductPages/ProductPageTemplate.test.tsx | 20 ++-- .../ProductPages/ProductPageTemplate.tsx | 10 +- .../ProductPages/ProgramAsCoursePage.test.tsx | 63 +++++++++++++ .../ProductPages/ProgramAsCoursePage.tsx | 11 ++- .../ProductPages/ProgramPage.test.tsx | 60 ++++++++++++ .../app-pages/ProductPages/ProgramPage.tsx | 11 ++- .../ProductPages/StayUpdatedModal.test.tsx | 3 +- .../ProductPages/test-utils/stayUpdated.ts | 53 +++++++++++ 10 files changed, 322 insertions(+), 13 deletions(-) create mode 100644 frontends/main/src/app-pages/ProductPages/test-utils/stayUpdated.ts diff --git a/frontends/main/src/app-pages/ProductPages/CoursePage.test.tsx b/frontends/main/src/app-pages/ProductPages/CoursePage.test.tsx index 70ed43908d..3a0e876afa 100644 --- a/frontends/main/src/app-pages/ProductPages/CoursePage.test.tsx +++ b/frontends/main/src/app-pages/ProductPages/CoursePage.test.tsx @@ -13,6 +13,7 @@ import { renderWithProviders, waitFor, screen, within } from "@/test-utils" import CoursePage from "./CoursePage" import { assertHeadings } from "ol-test-utilities" import { notFound } from "next/navigation" +import { useStayUpdatedEnv } from "./test-utils/stayUpdated" import { useFeatureFlagEnabled } from "posthog-js/react" import { useFeatureFlagsLoaded } from "@/common/useFeatureFlagsLoaded" @@ -267,4 +268,96 @@ describe("CoursePage", () => { expect(notFound).toHaveBeenCalled() }) }) + + describe("Stay Updated button", () => { + useStayUpdatedEnv() + + test("Shows button when all course runs have only the verified enrollment mode", async () => { + const verifiedMode = factories.courses.enrollmentMode({ + mode_slug: "verified", + }) + const course = makeCourse({ + courseruns: [ + factories.courses.courseRun({ enrollment_modes: [verifiedMode] }), + factories.courses.courseRun({ enrollment_modes: [verifiedMode] }), + ], + }) + const page = makePage({ course_details: course }) + setupApis({ course, page }) + renderWithProviders() + + expect( + await screen.findByRole("button", { name: "Stay Updated" }), + ).toBeInTheDocument() + }) + + test.each([ + { + label: "one run has a non-verified mode", + buildRuns: () => [ + factories.courses.courseRun({ + enrollment_modes: [ + factories.courses.enrollmentMode({ mode_slug: "verified" }), + ], + }), + factories.courses.courseRun({ + enrollment_modes: [ + factories.courses.enrollmentMode({ mode_slug: "audit" }), + ], + }), + ], + }, + { + label: "a run has mixed verified and non-verified modes", + buildRuns: () => [ + factories.courses.courseRun({ + enrollment_modes: [ + factories.courses.enrollmentMode({ mode_slug: "verified" }), + factories.courses.enrollmentMode({ mode_slug: "audit" }), + ], + }), + ], + }, + { + label: "a run has no enrollment modes", + buildRuns: () => [ + factories.courses.courseRun({ enrollment_modes: [] }), + ], + }, + { + label: "the course has no runs", + buildRuns: () => [], + }, + ])("Hides button when $label", async ({ buildRuns }) => { + const course = makeCourse({ courseruns: buildRuns() }) + const page = makePage({ course_details: course }) + setupApis({ course, page }) + renderWithProviders() + + await screen.findByRole("heading", { name: page.title }) + expect( + screen.queryByRole("button", { name: "Stay Updated" }), + ).not.toBeInTheDocument() + }) + + test("Hides button when Stay Updated form ID is not configured", async () => { + delete process.env.NEXT_PUBLIC_STAY_UPDATED_HUBSPOT_FORM_ID + const verifiedMode = factories.courses.enrollmentMode({ + mode_slug: "verified", + }) + const course = makeCourse({ + courseruns: [ + factories.courses.courseRun({ enrollment_modes: [verifiedMode] }), + ], + }) + const page = makePage({ course_details: course }) + setupApis({ course, page }) + renderWithProviders() + + await screen.findByRole("heading", { name: page.title }) + expect( + screen.queryByRole("button", { name: "Stay Updated" }), + ).not.toBeInTheDocument() + }) + }) }) diff --git a/frontends/main/src/app-pages/ProductPages/CoursePage.tsx b/frontends/main/src/app-pages/ProductPages/CoursePage.tsx index 15cf6d7f1d..b351d5c630 100644 --- a/frontends/main/src/app-pages/ProductPages/CoursePage.tsx +++ b/frontends/main/src/app-pages/ProductPages/CoursePage.tsx @@ -18,6 +18,7 @@ import ProductPageTemplate from "./ProductPageTemplate" import WhatYoullLearnSection from "./WhatYoullLearnSection" import HowYoullLearnSection, { DEFAULT_HOW_DATA } from "./HowYoullLearnSection" import { DEFAULT_RESOURCE_IMG } from "ol-utilities" +import { isVerifiedEnrollmentMode } from "@/common/mitxonline" import { useFeatureFlagsLoaded } from "@/common/useFeatureFlagsLoaded" import CourseInfoBox from "./InfoBoxCourse" import CourseEnrollmentButton from "./CourseEnrollmentButton" @@ -76,6 +77,16 @@ const CoursePage: React.FC = ({ readableId }) => { enrollmentAction={ } + showStayUpdated={ + course.courseruns.length > 0 && + course.courseruns.every( + (run) => + run.enrollment_modes.length > 0 && + run.enrollment_modes.every((mode) => + isVerifiedEnrollmentMode(mode.mode_slug), + ), + ) + } > {page.about ? ( diff --git a/frontends/main/src/app-pages/ProductPages/ProductPageTemplate.test.tsx b/frontends/main/src/app-pages/ProductPages/ProductPageTemplate.test.tsx index 3a8a393f85..65ba6d6986 100644 --- a/frontends/main/src/app-pages/ProductPages/ProductPageTemplate.test.tsx +++ b/frontends/main/src/app-pages/ProductPages/ProductPageTemplate.test.tsx @@ -4,6 +4,7 @@ import { renderWithProviders, screen } from "@/test-utils" import ProductPageTemplate from "./ProductPageTemplate" import { useHubspotFormDetail } from "api/hooks/hubspot" import NiceModal from "@ebay/nice-modal-react" +import { STAY_UPDATED_FORM_ID } from "./test-utils/stayUpdated" jest.mock("api/hooks/hubspot", () => ({ ...jest.requireActual("api/hooks/hubspot"), @@ -27,9 +28,11 @@ const mockedNiceModalShow = NiceModal.show as jest.MockedFunction< typeof NiceModal.show > -const STAY_UPDATED_FORM_ID = "4f423dc7-5b08-430b-a9fb-920b7f9597ed" - -const renderProductPageTemplate = () => { +const renderProductPageTemplate = ({ + showStayUpdated, +}: { + showStayUpdated?: boolean +} = {}) => { setMockResponse.get(urls.userMe.get(), { is_authenticated: false }) renderWithProviders( { imageSrc="/test-image.jpg" infoBox={
Info box
} enrollmentAction={} + showStayUpdated={showStayUpdated} >
Page content
, @@ -66,7 +70,9 @@ describe("ProductPageTemplate stay-updated trigger", () => { expect( screen.queryByRole("button", { name: "Stay Updated" }), ).not.toBeInTheDocument() - expect(mockedUseHubspotFormDetail).toHaveBeenCalledWith(undefined) + expect(mockedUseHubspotFormDetail).toHaveBeenCalledWith(undefined, { + enabled: false, + }) }) it("opens the modal when form id is configured even if the form is not yet fetched", () => { @@ -76,7 +82,7 @@ describe("ProductPageTemplate stay-updated trigger", () => { isError: false, } as ReturnType) - renderProductPageTemplate() + renderProductPageTemplate({ showStayUpdated: true }) const button = screen.getByRole("button", { name: "Stay Updated" }) expect(button).toBeInTheDocument() @@ -93,7 +99,7 @@ describe("ProductPageTemplate stay-updated trigger", () => { isError: true, } as ReturnType) - renderProductPageTemplate() + renderProductPageTemplate({ showStayUpdated: true }) const button = screen.getByRole("button", { name: "Stay Updated" }) expect(button).toBeInTheDocument() @@ -110,7 +116,7 @@ describe("ProductPageTemplate stay-updated trigger", () => { isError: false, } as unknown as ReturnType) - renderProductPageTemplate() + renderProductPageTemplate({ showStayUpdated: true }) const button = screen.getByRole("button", { name: "Stay Updated" }) expect(button).toBeInTheDocument() diff --git a/frontends/main/src/app-pages/ProductPages/ProductPageTemplate.tsx b/frontends/main/src/app-pages/ProductPages/ProductPageTemplate.tsx index da7001f22b..476750e8c9 100644 --- a/frontends/main/src/app-pages/ProductPages/ProductPageTemplate.tsx +++ b/frontends/main/src/app-pages/ProductPages/ProductPageTemplate.tsx @@ -263,6 +263,7 @@ type ProductPageTemplateProps = { infoBox: React.ReactNode enrollmentAction: React.ReactNode children: React.ReactNode + showStayUpdated?: boolean } const ProductPageTemplate: React.FC = ({ currentBreadcrumbLabel, @@ -273,13 +274,18 @@ const ProductPageTemplate: React.FC = ({ infoBox, children, enrollmentAction, + showStayUpdated, }) => { const stayUpdatedFormId = getStayUpdatedHubspotFormId() + const shouldShowStayUpdatedButton = Boolean( + stayUpdatedFormId && showStayUpdated, + ) const stayUpdatedParams = stayUpdatedFormId ? { form_id: stayUpdatedFormId } : undefined - const formQuery = useHubspotFormDetail(stayUpdatedParams) - const shouldShowStayUpdatedButton = Boolean(stayUpdatedFormId) + const formQuery = useHubspotFormDetail(stayUpdatedParams, { + enabled: shouldShowStayUpdatedButton, + }) return ( diff --git a/frontends/main/src/app-pages/ProductPages/ProgramAsCoursePage.test.tsx b/frontends/main/src/app-pages/ProductPages/ProgramAsCoursePage.test.tsx index 3ea3484713..188754fba3 100644 --- a/frontends/main/src/app-pages/ProductPages/ProgramAsCoursePage.test.tsx +++ b/frontends/main/src/app-pages/ProductPages/ProgramAsCoursePage.test.tsx @@ -20,6 +20,10 @@ import { assertHeadings } from "ol-test-utilities" import ProgramAsCoursePage from "./ProgramAsCoursePage" import { notFound } from "next/navigation" import { useFeatureFlagEnabled } from "posthog-js/react" +import { + useStayUpdatedEnv, + PROGRAM_HIDE_STAY_UPDATED_CASES, +} from "./test-utils/stayUpdated" import invariant from "tiny-invariant" import { useFeatureFlagsLoaded } from "@/common/useFeatureFlagsLoaded" import { getIdsFromReqTree } from "@/common/mitxonline" @@ -294,4 +298,63 @@ describe("ProgramAsCoursePage", () => { expect(listItems[1]).toHaveTextContent(childPrograms[0].title) expect(listItems[2]).toHaveTextContent(courses[1].title) }) + + describe("Stay Updated button", () => { + useStayUpdatedEnv() + + test("Shows button when program has only the verified enrollment mode", async () => { + const program = makeProgramAsCourse({ + enrollment_modes: [ + factories.courses.enrollmentMode({ mode_slug: "verified" }), + ], + }) + const page = makePage({ program_details: program }) + setupApis({ program, page }) + renderWithProviders( + , + ) + + expect( + await screen.findByRole("button", { name: "Stay Updated" }), + ).toBeInTheDocument() + }) + + test.each(PROGRAM_HIDE_STAY_UPDATED_CASES)( + "Hides button when $label", + async ({ enrollment_modes: enrollmentModes }) => { + const program = makeProgramAsCourse({ + enrollment_modes: enrollmentModes, + }) + const page = makePage({ program_details: program }) + setupApis({ program, page }) + renderWithProviders( + , + ) + + await screen.findByRole("heading", { name: page.title }) + expect( + screen.queryByRole("button", { name: "Stay Updated" }), + ).not.toBeInTheDocument() + }, + ) + + test("Hides button when Stay Updated form ID is not configured", async () => { + delete process.env.NEXT_PUBLIC_STAY_UPDATED_HUBSPOT_FORM_ID + const program = makeProgramAsCourse({ + enrollment_modes: [ + factories.courses.enrollmentMode({ mode_slug: "verified" }), + ], + }) + const page = makePage({ program_details: program }) + setupApis({ program, page }) + renderWithProviders( + , + ) + + await screen.findByRole("heading", { name: page.title }) + expect( + screen.queryByRole("button", { name: "Stay Updated" }), + ).not.toBeInTheDocument() + }) + }) }) diff --git a/frontends/main/src/app-pages/ProductPages/ProgramAsCoursePage.tsx b/frontends/main/src/app-pages/ProductPages/ProgramAsCoursePage.tsx index 155c54da58..d51e9c9ff6 100644 --- a/frontends/main/src/app-pages/ProductPages/ProgramAsCoursePage.tsx +++ b/frontends/main/src/app-pages/ProductPages/ProgramAsCoursePage.tsx @@ -11,7 +11,10 @@ import { FeatureFlags } from "@/common/feature_flags" import { notFound } from "next/navigation" import { HeadingIds, parseReqTree } from "./util" import type { RequirementItem } from "./util" -import { getIdsFromReqTree } from "@/common/mitxonline" +import { + getIdsFromReqTree, + isVerifiedEnrollmentMode, +} from "@/common/mitxonline" import InstructorsSection from "./InstructorsSection" import RawHTML from "./RawHTML" import UnstyledRawHTML from "@/components/UnstyledRawHTML/UnstyledRawHTML" @@ -280,6 +283,12 @@ const ProgramAsCoursePage: React.FC = ({ displayAsCourse /> } + showStayUpdated={ + program.enrollment_modes.length > 0 && + program.enrollment_modes.every((mode) => + isVerifiedEnrollmentMode(mode.mode_slug), + ) + } infoBox={ { expect(notFound).toHaveBeenCalled() }) }) + + describe("Stay Updated button", () => { + useStayUpdatedEnv() + + test("Shows button when program has only the verified enrollment mode", async () => { + const program = makeProgram({ + ...makeReqs(), + enrollment_modes: [ + factories.courses.enrollmentMode({ mode_slug: "verified" }), + ], + }) + const page = makePage({ program_details: program }) + setupApis({ program, page }) + renderWithProviders() + + expect( + await screen.findByRole("button", { name: "Stay Updated" }), + ).toBeInTheDocument() + }) + + test.each(PROGRAM_HIDE_STAY_UPDATED_CASES)( + "Hides button when $label", + async ({ enrollment_modes: enrollmentModes }) => { + const program = makeProgram({ + ...makeReqs(), + enrollment_modes: enrollmentModes, + }) + const page = makePage({ program_details: program }) + setupApis({ program, page }) + renderWithProviders() + + await screen.findByRole("heading", { name: page.title }) + expect( + screen.queryByRole("button", { name: "Stay Updated" }), + ).not.toBeInTheDocument() + }, + ) + + test("Hides button when Stay Updated form ID is not configured", async () => { + delete process.env.NEXT_PUBLIC_STAY_UPDATED_HUBSPOT_FORM_ID + const program = makeProgram({ + ...makeReqs(), + enrollment_modes: [ + factories.courses.enrollmentMode({ mode_slug: "verified" }), + ], + }) + const page = makePage({ program_details: program }) + setupApis({ program, page }) + renderWithProviders() + + await screen.findByRole("heading", { name: page.title }) + expect( + screen.queryByRole("button", { name: "Stay Updated" }), + ).not.toBeInTheDocument() + }) + }) }) diff --git a/frontends/main/src/app-pages/ProductPages/ProgramPage.tsx b/frontends/main/src/app-pages/ProductPages/ProgramPage.tsx index 01042acff8..e17659d664 100644 --- a/frontends/main/src/app-pages/ProductPages/ProgramPage.tsx +++ b/frontends/main/src/app-pages/ProductPages/ProgramPage.tsx @@ -11,7 +11,10 @@ import { useFeatureFlagEnabled } from "posthog-js/react" import { FeatureFlags } from "@/common/feature_flags" import { notFound } from "next/navigation" import { HeadingIds, parseReqTree, RequirementData } from "./util" -import { getIdsFromReqTree } from "@/common/mitxonline" +import { + getIdsFromReqTree, + isVerifiedEnrollmentMode, +} from "@/common/mitxonline" import InstructorsSection from "./InstructorsSection" import RawHTML from "./RawHTML" import UnstyledRawHTML from "@/components/UnstyledRawHTML/UnstyledRawHTML" @@ -279,6 +282,12 @@ const ProgramPage: React.FC = ({ readableId }) => { enrollmentAction={ } + showStayUpdated={ + program.enrollment_modes.length > 0 && + program.enrollment_modes.every((mode) => + isVerifiedEnrollmentMode(mode.mode_slug), + ) + } infoBox={ } diff --git a/frontends/main/src/app-pages/ProductPages/StayUpdatedModal.test.tsx b/frontends/main/src/app-pages/ProductPages/StayUpdatedModal.test.tsx index c2ba27ceaa..c912c9fd1b 100644 --- a/frontends/main/src/app-pages/ProductPages/StayUpdatedModal.test.tsx +++ b/frontends/main/src/app-pages/ProductPages/StayUpdatedModal.test.tsx @@ -4,6 +4,7 @@ import { HubspotForm, type HubspotFormProps } from "ol-components" import { setMockResponse, urls, factories } from "api/test-utils" import { renderWithProviders, screen, user, act } from "@/test-utils" import { StayUpdatedModal } from "./StayUpdatedModal" +import { STAY_UPDATED_FORM_ID } from "./test-utils/stayUpdated" jest.mock("ol-components", () => ({ ...jest.requireActual("ol-components"), @@ -11,8 +12,6 @@ jest.mock("ol-components", () => ({ })) const mockedHubspotForm = jest.mocked(HubspotForm) - -const STAY_UPDATED_FORM_ID = "4f423dc7-5b08-430b-a9fb-920b7f9597ed" const TEST_EMAIL = "user@test.edu" const setupApis = () => { diff --git a/frontends/main/src/app-pages/ProductPages/test-utils/stayUpdated.ts b/frontends/main/src/app-pages/ProductPages/test-utils/stayUpdated.ts new file mode 100644 index 0000000000..e070b06152 --- /dev/null +++ b/frontends/main/src/app-pages/ProductPages/test-utils/stayUpdated.ts @@ -0,0 +1,53 @@ +import { factories } from "api/mitxonline-test-utils" +import type { EnrollmentMode } from "@mitodl/mitxonline-api-axios/v2" +import { faker } from "@faker-js/faker/locale/en" + +export const STAY_UPDATED_FORM_ID = faker.string.uuid() + +/** + * Sets the Stay Updated Hubspot form ID env var before each test and removes + * it after. Call inside a describe block. + */ +export const useStayUpdatedEnv = () => { + let previousFormId: string | undefined + + beforeEach(() => { + previousFormId = process.env.NEXT_PUBLIC_STAY_UPDATED_HUBSPOT_FORM_ID + process.env.NEXT_PUBLIC_STAY_UPDATED_HUBSPOT_FORM_ID = STAY_UPDATED_FORM_ID + }) + + afterEach(() => { + if (previousFormId === undefined) { + delete process.env.NEXT_PUBLIC_STAY_UPDATED_HUBSPOT_FORM_ID + } else { + process.env.NEXT_PUBLIC_STAY_UPDATED_HUBSPOT_FORM_ID = previousFormId + } + }) +} + +/** + * Shared test.each cases for the "Stay Updated button is hidden" scenarios on + * program-level enrollment_modes (used by ProgramPage and ProgramAsCoursePage). + */ +export const PROGRAM_HIDE_STAY_UPDATED_CASES: { + label: string + enrollment_modes: EnrollmentMode[] +}[] = [ + { + label: "program has a non-verified enrollment mode", + enrollment_modes: [ + factories.courses.enrollmentMode({ mode_slug: "audit" }), + ], + }, + { + label: "program has mixed verified and non-verified modes", + enrollment_modes: [ + factories.courses.enrollmentMode({ mode_slug: "verified" }), + factories.courses.enrollmentMode({ mode_slug: "audit" }), + ], + }, + { + label: "program has no enrollment modes", + enrollment_modes: [], + }, +] From 704a2ed4677c32ace9eea290a587e294421317a9 Mon Sep 17 00:00:00 2001 From: Shankar Ambady Date: Wed, 15 Apr 2026 15:22:38 -0400 Subject: [PATCH 4/5] Revert "Facet counts and aggregations for Vector search (#3188)" (#3208) This reverts commit e6f41cb8b315d3de0782a1871aa6e426b62dc83f. --- frontends/api/src/generated/v0/api.ts | 105 -------- .../app-pages/SearchPage/SearchPage.test.tsx | 44 ++++ .../SearchDisplay/SearchDisplay.tsx | 18 +- main/settings.py | 4 +- openapi/specs/v0.yaml | 114 -------- vector_search/constants.py | 14 - vector_search/serializers.py | 45 +--- vector_search/utils.py | 74 +----- vector_search/utils_test.py | 246 ------------------ vector_search/views.py | 90 ++----- vector_search/views_test.py | 12 +- 11 files changed, 98 insertions(+), 668 deletions(-) diff --git a/frontends/api/src/generated/v0/api.ts b/frontends/api/src/generated/v0/api.ts index 8e0eb0472a..368c911b52 100644 --- a/frontends/api/src/generated/v0/api.ts +++ b/frontends/api/src/generated/v0/api.ts @@ -11532,7 +11532,6 @@ export const VectorContentFilesSearchApiAxiosParamCreator = function ( /** * Vector Search for content * @summary Content File Vector Search - * @param {Array} [aggregations] aggregations for facet counts * `key` - Key * `course_number` - Course Number * `platform` - Platform * `offered_by` - Offered By * `file_extension` - File Extension * `content_feature_type` - Content Feature Type * `run_readable_id` - Run Readable Id * `resource_readable_id` - Resource Readable Id * `run_title` - Run Title * `edx_module_id` - Edx Module Id * `content_type` - Content Type * `description` - Description * `title` - Title * `url` - Url * `file_type` - File Type * `summary` - Summary * `flashcards` - Flashcards * `checksum` - Checksum * @param {string} [collection_name] Manually specify the name of the Qdrant collection to query * @param {Array} [file_extension] The extension of the content file. * @param {string} [group_by] The attribute to group results by @@ -11553,7 +11552,6 @@ export const VectorContentFilesSearchApiAxiosParamCreator = function ( * @throws {RequiredError} */ vectorContentFilesSearchRetrieve: async ( - aggregations?: Array, collection_name?: string, file_extension?: Array, group_by?: string, @@ -11588,10 +11586,6 @@ export const VectorContentFilesSearchApiAxiosParamCreator = function ( const localVarHeaderParameter = {} as any const localVarQueryParameter = {} as any - if (aggregations) { - localVarQueryParameter["aggregations"] = aggregations - } - if (collection_name !== undefined) { localVarQueryParameter["collection_name"] = collection_name } @@ -11686,7 +11680,6 @@ export const VectorContentFilesSearchApiFp = function ( /** * Vector Search for content * @summary Content File Vector Search - * @param {Array} [aggregations] aggregations for facet counts * `key` - Key * `course_number` - Course Number * `platform` - Platform * `offered_by` - Offered By * `file_extension` - File Extension * `content_feature_type` - Content Feature Type * `run_readable_id` - Run Readable Id * `resource_readable_id` - Resource Readable Id * `run_title` - Run Title * `edx_module_id` - Edx Module Id * `content_type` - Content Type * `description` - Description * `title` - Title * `url` - Url * `file_type` - File Type * `summary` - Summary * `flashcards` - Flashcards * `checksum` - Checksum * @param {string} [collection_name] Manually specify the name of the Qdrant collection to query * @param {Array} [file_extension] The extension of the content file. * @param {string} [group_by] The attribute to group results by @@ -11707,7 +11700,6 @@ export const VectorContentFilesSearchApiFp = function ( * @throws {RequiredError} */ async vectorContentFilesSearchRetrieve( - aggregations?: Array, collection_name?: string, file_extension?: Array, group_by?: string, @@ -11733,7 +11725,6 @@ export const VectorContentFilesSearchApiFp = function ( > { const localVarAxiosArgs = await localVarAxiosParamCreator.vectorContentFilesSearchRetrieve( - aggregations, collection_name, file_extension, group_by, @@ -11792,7 +11783,6 @@ export const VectorContentFilesSearchApiFactory = function ( ): AxiosPromise { return localVarFp .vectorContentFilesSearchRetrieve( - requestParameters.aggregations, requestParameters.collection_name, requestParameters.file_extension, requestParameters.group_by, @@ -11822,13 +11812,6 @@ export const VectorContentFilesSearchApiFactory = function ( * @interface VectorContentFilesSearchApiVectorContentFilesSearchRetrieveRequest */ export interface VectorContentFilesSearchApiVectorContentFilesSearchRetrieveRequest { - /** - * aggregations for facet counts * `key` - Key * `course_number` - Course Number * `platform` - Platform * `offered_by` - Offered By * `file_extension` - File Extension * `content_feature_type` - Content Feature Type * `run_readable_id` - Run Readable Id * `resource_readable_id` - Resource Readable Id * `run_title` - Run Title * `edx_module_id` - Edx Module Id * `content_type` - Content Type * `description` - Description * `title` - Title * `url` - Url * `file_type` - File Type * `summary` - Summary * `flashcards` - Flashcards * `checksum` - Checksum - * @type {Array<'key' | 'course_number' | 'platform' | 'offered_by' | 'file_extension' | 'content_feature_type' | 'run_readable_id' | 'resource_readable_id' | 'run_title' | 'edx_module_id' | 'content_type' | 'description' | 'title' | 'url' | 'file_type' | 'summary' | 'flashcards' | 'checksum'>} - * @memberof VectorContentFilesSearchApiVectorContentFilesSearchRetrieve - */ - readonly aggregations?: Array - /** * Manually specify the name of the Qdrant collection to query * @type {string} @@ -11963,7 +11946,6 @@ export class VectorContentFilesSearchApi extends BaseAPI { ) { return VectorContentFilesSearchApiFp(this.configuration) .vectorContentFilesSearchRetrieve( - requestParameters.aggregations, requestParameters.collection_name, requestParameters.file_extension, requestParameters.group_by, @@ -11986,31 +11968,6 @@ export class VectorContentFilesSearchApi extends BaseAPI { } } -/** - * @export - */ -export const VectorContentFilesSearchRetrieveAggregationsEnum = { - Key: "key", - CourseNumber: "course_number", - Platform: "platform", - OfferedBy: "offered_by", - FileExtension: "file_extension", - ContentFeatureType: "content_feature_type", - RunReadableId: "run_readable_id", - ResourceReadableId: "resource_readable_id", - RunTitle: "run_title", - EdxModuleId: "edx_module_id", - ContentType: "content_type", - Description: "description", - Title: "title", - Url: "url", - FileType: "file_type", - Summary: "summary", - Flashcards: "flashcards", - Checksum: "checksum", -} as const -export type VectorContentFilesSearchRetrieveAggregationsEnum = - (typeof VectorContentFilesSearchRetrieveAggregationsEnum)[keyof typeof VectorContentFilesSearchRetrieveAggregationsEnum] /** * @export */ @@ -12034,7 +11991,6 @@ export const VectorLearningResourcesSearchApiAxiosParamCreator = function ( /** * Vector Search for learning resources * @summary Vector Search - * @param {Array} [aggregations] aggregations for facet counts * `readable_id` - Readable Id * `resource_type` - Resource Type * `certification` - Certification * `certification_type` - Certification Type * `professional` - Professional * `free` - Free * `course_feature` - Course Feature * `topic` - Topic * `ocw_topic` - Ocw Topic * `level` - Level * `department` - Department * `platform` - Platform * `offered_by` - Offered By * `delivery` - Delivery * `title` - Title * `url` - Url * `resource_type_group` - Resource Type Group * `resource_category` - Resource Category * `published` - Published * @param {boolean | null} [certification] True if the learning resource offers a certificate * @param {Array} [certification_type] The type of certificate * `micromasters` - MicroMasters Credential * `professional` - Professional Certificate * `completion` - Certificate of Completion * `none` - No Certificate * @param {Array} [course_feature] The course feature. Possible options are at api/v1/course_features/ @@ -12049,7 +12005,6 @@ export const VectorLearningResourcesSearchApiAxiosParamCreator = function ( * @param {number} [offset] The initial index from which to return the results * @param {Array} [platform] The platform on which the learning resource is offered * `edx` - edX * `ocw` - MIT OpenCourseWare * `oll` - Open Learning Library * `mitxonline` - MITx Online * `bootcamps` - Bootcamps * `xpro` - MIT xPRO * `csail` - CSAIL * `mitpe` - MIT Professional Education * `see` - MIT Sloan Executive Education * `scc` - Schwarzman College of Computing * `ctl` - Center for Transportation & Logistics * `whu` - WHU * `susskind` - Susskind * `globalalumni` - Global Alumni * `simplilearn` - Simplilearn * `emeritus` - Emeritus * `podcast` - Podcast * `youtube` - YouTube * `canvas` - Canvas * `climate` - MIT Climate * `ovs` - ODL Video Service * @param {boolean | null} [professional] - * @param {boolean} [published] If the resource is published. We default to True unless passed in * @param {string} [q] The search text * @param {string} [readable_id] The readable id of the resource * @param {Array} [resource_type] The type of learning resource * `course` - course * `program` - program * `learning_path` - learning path * `podcast` - podcast * `podcast_episode` - podcast episode * `video` - video * `video_playlist` - video playlist * `document` - document @@ -12061,7 +12016,6 @@ export const VectorLearningResourcesSearchApiAxiosParamCreator = function ( * @throws {RequiredError} */ vectorLearningResourcesSearchRetrieve: async ( - aggregations?: Array, certification?: boolean | null, certification_type?: Array, course_feature?: Array, @@ -12076,7 +12030,6 @@ export const VectorLearningResourcesSearchApiAxiosParamCreator = function ( offset?: number, platform?: Array, professional?: boolean | null, - published?: boolean, q?: string, readable_id?: string, resource_type?: Array, @@ -12102,10 +12055,6 @@ export const VectorLearningResourcesSearchApiAxiosParamCreator = function ( const localVarHeaderParameter = {} as any const localVarQueryParameter = {} as any - if (aggregations) { - localVarQueryParameter["aggregations"] = aggregations - } - if (certification !== undefined) { localVarQueryParameter["certification"] = certification } @@ -12162,10 +12111,6 @@ export const VectorLearningResourcesSearchApiAxiosParamCreator = function ( localVarQueryParameter["professional"] = professional } - if (published !== undefined) { - localVarQueryParameter["published"] = published - } - if (q !== undefined) { localVarQueryParameter["q"] = q } @@ -12224,7 +12169,6 @@ export const VectorLearningResourcesSearchApiFp = function ( /** * Vector Search for learning resources * @summary Vector Search - * @param {Array} [aggregations] aggregations for facet counts * `readable_id` - Readable Id * `resource_type` - Resource Type * `certification` - Certification * `certification_type` - Certification Type * `professional` - Professional * `free` - Free * `course_feature` - Course Feature * `topic` - Topic * `ocw_topic` - Ocw Topic * `level` - Level * `department` - Department * `platform` - Platform * `offered_by` - Offered By * `delivery` - Delivery * `title` - Title * `url` - Url * `resource_type_group` - Resource Type Group * `resource_category` - Resource Category * `published` - Published * @param {boolean | null} [certification] True if the learning resource offers a certificate * @param {Array} [certification_type] The type of certificate * `micromasters` - MicroMasters Credential * `professional` - Professional Certificate * `completion` - Certificate of Completion * `none` - No Certificate * @param {Array} [course_feature] The course feature. Possible options are at api/v1/course_features/ @@ -12239,7 +12183,6 @@ export const VectorLearningResourcesSearchApiFp = function ( * @param {number} [offset] The initial index from which to return the results * @param {Array} [platform] The platform on which the learning resource is offered * `edx` - edX * `ocw` - MIT OpenCourseWare * `oll` - Open Learning Library * `mitxonline` - MITx Online * `bootcamps` - Bootcamps * `xpro` - MIT xPRO * `csail` - CSAIL * `mitpe` - MIT Professional Education * `see` - MIT Sloan Executive Education * `scc` - Schwarzman College of Computing * `ctl` - Center for Transportation & Logistics * `whu` - WHU * `susskind` - Susskind * `globalalumni` - Global Alumni * `simplilearn` - Simplilearn * `emeritus` - Emeritus * `podcast` - Podcast * `youtube` - YouTube * `canvas` - Canvas * `climate` - MIT Climate * `ovs` - ODL Video Service * @param {boolean | null} [professional] - * @param {boolean} [published] If the resource is published. We default to True unless passed in * @param {string} [q] The search text * @param {string} [readable_id] The readable id of the resource * @param {Array} [resource_type] The type of learning resource * `course` - course * `program` - program * `learning_path` - learning path * `podcast` - podcast * `podcast_episode` - podcast episode * `video` - video * `video_playlist` - video playlist * `document` - document @@ -12251,7 +12194,6 @@ export const VectorLearningResourcesSearchApiFp = function ( * @throws {RequiredError} */ async vectorLearningResourcesSearchRetrieve( - aggregations?: Array, certification?: boolean | null, certification_type?: Array, course_feature?: Array, @@ -12266,7 +12208,6 @@ export const VectorLearningResourcesSearchApiFp = function ( offset?: number, platform?: Array, professional?: boolean | null, - published?: boolean, q?: string, readable_id?: string, resource_type?: Array, @@ -12283,7 +12224,6 @@ export const VectorLearningResourcesSearchApiFp = function ( > { const localVarAxiosArgs = await localVarAxiosParamCreator.vectorLearningResourcesSearchRetrieve( - aggregations, certification, certification_type, course_feature, @@ -12298,7 +12238,6 @@ export const VectorLearningResourcesSearchApiFp = function ( offset, platform, professional, - published, q, readable_id, resource_type, @@ -12348,7 +12287,6 @@ export const VectorLearningResourcesSearchApiFactory = function ( ): AxiosPromise { return localVarFp .vectorLearningResourcesSearchRetrieve( - requestParameters.aggregations, requestParameters.certification, requestParameters.certification_type, requestParameters.course_feature, @@ -12363,7 +12301,6 @@ export const VectorLearningResourcesSearchApiFactory = function ( requestParameters.offset, requestParameters.platform, requestParameters.professional, - requestParameters.published, requestParameters.q, requestParameters.readable_id, requestParameters.resource_type, @@ -12384,13 +12321,6 @@ export const VectorLearningResourcesSearchApiFactory = function ( * @interface VectorLearningResourcesSearchApiVectorLearningResourcesSearchRetrieveRequest */ export interface VectorLearningResourcesSearchApiVectorLearningResourcesSearchRetrieveRequest { - /** - * aggregations for facet counts * `readable_id` - Readable Id * `resource_type` - Resource Type * `certification` - Certification * `certification_type` - Certification Type * `professional` - Professional * `free` - Free * `course_feature` - Course Feature * `topic` - Topic * `ocw_topic` - Ocw Topic * `level` - Level * `department` - Department * `platform` - Platform * `offered_by` - Offered By * `delivery` - Delivery * `title` - Title * `url` - Url * `resource_type_group` - Resource Type Group * `resource_category` - Resource Category * `published` - Published - * @type {Array<'readable_id' | 'resource_type' | 'certification' | 'certification_type' | 'professional' | 'free' | 'course_feature' | 'topic' | 'ocw_topic' | 'level' | 'department' | 'platform' | 'offered_by' | 'delivery' | 'title' | 'url' | 'resource_type_group' | 'resource_category' | 'published'>} - * @memberof VectorLearningResourcesSearchApiVectorLearningResourcesSearchRetrieve - */ - readonly aggregations?: Array - /** * True if the learning resource offers a certificate * @type {boolean} @@ -12489,13 +12419,6 @@ export interface VectorLearningResourcesSearchApiVectorLearningResourcesSearchRe */ readonly professional?: boolean | null - /** - * If the resource is published. We default to True unless passed in - * @type {boolean} - * @memberof VectorLearningResourcesSearchApiVectorLearningResourcesSearchRetrieve - */ - readonly published?: boolean - /** * The search text * @type {string} @@ -12567,7 +12490,6 @@ export class VectorLearningResourcesSearchApi extends BaseAPI { ) { return VectorLearningResourcesSearchApiFp(this.configuration) .vectorLearningResourcesSearchRetrieve( - requestParameters.aggregations, requestParameters.certification, requestParameters.certification_type, requestParameters.course_feature, @@ -12582,7 +12504,6 @@ export class VectorLearningResourcesSearchApi extends BaseAPI { requestParameters.offset, requestParameters.platform, requestParameters.professional, - requestParameters.published, requestParameters.q, requestParameters.readable_id, requestParameters.resource_type, @@ -12596,32 +12517,6 @@ export class VectorLearningResourcesSearchApi extends BaseAPI { } } -/** - * @export - */ -export const VectorLearningResourcesSearchRetrieveAggregationsEnum = { - ReadableId: "readable_id", - ResourceType: "resource_type", - Certification: "certification", - CertificationType: "certification_type", - Professional: "professional", - Free: "free", - CourseFeature: "course_feature", - Topic: "topic", - OcwTopic: "ocw_topic", - Level: "level", - Department: "department", - Platform: "platform", - OfferedBy: "offered_by", - Delivery: "delivery", - Title: "title", - Url: "url", - ResourceTypeGroup: "resource_type_group", - ResourceCategory: "resource_category", - Published: "published", -} as const -export type VectorLearningResourcesSearchRetrieveAggregationsEnum = - (typeof VectorLearningResourcesSearchRetrieveAggregationsEnum)[keyof typeof VectorLearningResourcesSearchRetrieveAggregationsEnum] /** * @export */ diff --git a/frontends/main/src/app-pages/SearchPage/SearchPage.test.tsx b/frontends/main/src/app-pages/SearchPage/SearchPage.test.tsx index 2d9cb44807..fe14c1c100 100644 --- a/frontends/main/src/app-pages/SearchPage/SearchPage.test.tsx +++ b/frontends/main/src/app-pages/SearchPage/SearchPage.test.tsx @@ -152,6 +152,50 @@ describe("SearchPage", () => { }, ) + test("Vector Hybrid Search passes correct params and hides count", async () => { + setMockApiResponses({ + search: { + count: 700, + metadata: { + aggregations: { + resource_type_group: [{ key: "course", doc_count: 100 }], + }, + suggestions: [], + }, + results: factories.learningResources.resources({ count: 5 }).results, + }, + }) + + // Authenticate as path editor (admin) + setMockResponse.get(urls.userMe.get(), { + is_learning_path_editor: true, + is_authenticated: true, + }) + + renderWithProviders(, { url: "?vector_search=true&q=test" }) + + await waitFor(() => { + const call = makeRequest.mock.calls.find(([_method, url]) => { + return url.includes(urls.search.vectorResources()) + }) + expect(call).toBeDefined() + }) + + const call = makeRequest.mock.calls.find(([_method, url]) => + url.includes(urls.search.vectorResources()), + ) + invariant(call) + const fullUrl = new URL(call[1], "http://mit.edu") + const apiSearchParams = fullUrl.searchParams + + expect(apiSearchParams.get("hybrid_search")).toBe("true") + expect(apiSearchParams.get("q")).toBe("test") + + // Ensure count is hidden + const hideCountText = screen.queryByText("700 results") + expect(hideCountText).toBeNull() + }) + test("Toggling facets", async () => { setMockApiResponses({ search: { diff --git a/frontends/main/src/page-components/SearchDisplay/SearchDisplay.tsx b/frontends/main/src/page-components/SearchDisplay/SearchDisplay.tsx index e0b53a8ee8..2cf28c753e 100644 --- a/frontends/main/src/page-components/SearchDisplay/SearchDisplay.tsx +++ b/frontends/main/src/page-components/SearchDisplay/SearchDisplay.tsx @@ -516,8 +516,8 @@ const searchModeDropdownOptions = Object.entries( /** * Extracts only the fields supported by the vector search API from a broader - * search params object, dropping admin-only params (e.g., content_file_score_weight) - * that the vector endpoint does not accept. + * search params object, dropping admin-only params (e.g., aggregations, + * content_file_score_weight) that the vector endpoint does not accept. * * The `as` casts for enum arrays are safe because the v0 and v1 generated * clients define separate (but structurally identical) enum types for the same @@ -526,7 +526,6 @@ const searchModeDropdownOptions = Object.entries( const toVectorSearchParams = ( params: ReturnType, ): VectorSearchRequest => ({ - aggregations: params.aggregations as VectorSearchRequest["aggregations"], certification: params.certification, certification_type: params.certification_type as VectorSearchRequest["certification_type"], @@ -626,13 +625,10 @@ const SearchDisplay: React.FC = ({ const wantsVectorSearch = searchParams.get("vector_search") === "true" const isVectorSearch = wantsVectorSearch && user?.is_learning_path_editor - const queryOptions = isVectorSearch - ? learningResourceQueries.vectorSearch(toVectorSearchParams(allParams)) - : learningResourceQueries.search(allParams as LRSearchRequest) - - // @ts-expect-error Typescript has trouble unifying the different query key types const { data, isLoading, isFetching } = useQuery({ - ...queryOptions, + ...(isVectorSearch + ? learningResourceQueries.vectorSearch(toVectorSearchParams(allParams)) + : learningResourceQueries.search(allParams as LRSearchRequest)), enabled: !wantsVectorSearch || !isUserLoading, placeholderData: keepPreviousData, select: (timedData: { @@ -989,7 +985,9 @@ const SearchDisplay: React.FC = ({ * the count when data is loaded even if count is same as previous * count. */} - {isFetching || isLoading ? "" : `${data?.count} results`} + {isFetching || isLoading || isVectorSearch + ? "" + : `${data?.count} results`} diff --git a/main/settings.py b/main/settings.py index f658f33bac..ca08d99998 100644 --- a/main/settings.py +++ b/main/settings.py @@ -822,10 +822,10 @@ def get_all_config_keys(): QDRANT_CLIENT_TIMEOUT = get_int(name="QDRANT_CLIENT_TIMEOUT", default=10) VECTOR_HYBRID_SEARCH_PREFETCH_MULTIPLIER = get_int( - name="VECTOR_HYBRID_SEARCH_PREFETCH_MULTIPLIER", default=5 + name="VECTOR_HYBRID_SEARCH_PREFETCH_MULTIPLIER", default=20 ) VECTOR_HYBRID_SEARCH_PREFETCH_MAX_LIMIT = get_int( - name="VECTOR_HYBRID_SEARCH_PREFETCH_MAX_LIMIT", default=500 + name="VECTOR_HYBRID_SEARCH_PREFETCH_MAX_LIMIT", default=10000 ) # toggle to use requests (default for local) or webdriver which renders js elements EMBEDDINGS_EXTERNAL_FETCH_USE_WEBDRIVER = get_bool( diff --git a/openapi/specs/v0.yaml b/openapi/specs/v0.yaml index 85bfabe127..4bb97c19c2 100644 --- a/openapi/specs/v0.yaml +++ b/openapi/specs/v0.yaml @@ -827,58 +827,6 @@ paths: description: Vector Search for content summary: Content File Vector Search parameters: - - in: query - name: aggregations - schema: - type: array - items: - enum: - - key - - course_number - - platform - - offered_by - - file_extension - - content_feature_type - - run_readable_id - - resource_readable_id - - run_title - - edx_module_id - - content_type - - description - - title - - url - - file_type - - summary - - flashcards - - checksum - type: string - description: |- - * `key` - Key - * `course_number` - Course Number - * `platform` - Platform - * `offered_by` - Offered By - * `file_extension` - File Extension - * `content_feature_type` - Content Feature Type - * `run_readable_id` - Run Readable Id - * `resource_readable_id` - Resource Readable Id - * `run_title` - Run Title - * `edx_module_id` - Edx Module Id - * `content_type` - Content Type - * `description` - Description - * `title` - Title - * `url` - Url - * `file_type` - File Type - * `summary` - Summary - * `flashcards` - Flashcards - * `checksum` - Checksum - description: "aggregations for facet counts \n\n* `key` - Key\n\ - * `course_number` - Course Number\n* `platform` - Platform\n* `offered_by`\ - \ - Offered By\n* `file_extension` - File Extension\n* `content_feature_type`\ - \ - Content Feature Type\n* `run_readable_id` - Run Readable Id\n* `resource_readable_id`\ - \ - Resource Readable Id\n* `run_title` - Run Title\n* `edx_module_id` -\ - \ Edx Module Id\n* `content_type` - Content Type\n* `description` - Description\n\ - * `title` - Title\n* `url` - Url\n* `file_type` - File Type\n* `summary`\ - \ - Summary\n* `flashcards` - Flashcards\n* `checksum` - Checksum" - in: query name: collection_name schema: @@ -1013,61 +961,6 @@ paths: description: Vector Search for learning resources summary: Vector Search parameters: - - in: query - name: aggregations - schema: - type: array - items: - enum: - - readable_id - - resource_type - - certification - - certification_type - - professional - - free - - course_feature - - topic - - ocw_topic - - level - - department - - platform - - offered_by - - delivery - - title - - url - - resource_type_group - - resource_category - - published - type: string - description: |- - * `readable_id` - Readable Id - * `resource_type` - Resource Type - * `certification` - Certification - * `certification_type` - Certification Type - * `professional` - Professional - * `free` - Free - * `course_feature` - Course Feature - * `topic` - Topic - * `ocw_topic` - Ocw Topic - * `level` - Level - * `department` - Department - * `platform` - Platform - * `offered_by` - Offered By - * `delivery` - Delivery - * `title` - Title - * `url` - Url - * `resource_type_group` - Resource Type Group - * `resource_category` - Resource Category - * `published` - Published - description: "aggregations for facet counts \n\n* `readable_id`\ - \ - Readable Id\n* `resource_type` - Resource Type\n* `certification` -\ - \ Certification\n* `certification_type` - Certification Type\n* `professional`\ - \ - Professional\n* `free` - Free\n* `course_feature` - Course Feature\n\ - * `topic` - Topic\n* `ocw_topic` - Ocw Topic\n* `level` - Level\n* `department`\ - \ - Department\n* `platform` - Platform\n* `offered_by` - Offered By\n*\ - \ `delivery` - Delivery\n* `title` - Title\n* `url` - Url\n* `resource_type_group`\ - \ - Resource Type Group\n* `resource_category` - Resource Category\n* `published`\ - \ - Published" - in: query name: certification schema: @@ -1362,13 +1255,6 @@ paths: schema: type: boolean nullable: true - - in: query - name: published - schema: - type: boolean - default: true - description: If the resource is published. We default to True unless passed - in - in: query name: q schema: diff --git a/vector_search/constants.py b/vector_search/constants.py index cadd81622a..0adc4f31a4 100644 --- a/vector_search/constants.py +++ b/vector_search/constants.py @@ -45,8 +45,6 @@ "title": "title", "url": "url", "resource_type_group": "resource_type_group", - "resource_category": "resource_category", - "published": "published", } @@ -73,7 +71,6 @@ "url": models.PayloadSchemaType.KEYWORD, "title": models.PayloadSchemaType.KEYWORD, "resource_type_group": models.PayloadSchemaType.KEYWORD, - "resource_category": models.PayloadSchemaType.KEYWORD, } """ @@ -95,14 +92,3 @@ QDRANT_TOPIC_INDEXES = { "name": models.PayloadSchemaType.KEYWORD, } - - -CONTENT_FILES_RETRIEVE_PAYLOAD = ["key", "run_readable_id"] -RESOURCES_RETRIEVE_PAYLOAD = ["readable_id"] - - -COLLECTION_PARAM_MAP = { - RESOURCES_COLLECTION_NAME: QDRANT_RESOURCE_PARAM_MAP, - TOPICS_COLLECTION_NAME: QDRANT_TOPICS_PARAM_MAP, - CONTENT_FILES_COLLECTION_NAME: QDRANT_CONTENT_FILE_PARAM_MAP, -} diff --git a/vector_search/serializers.py b/vector_search/serializers.py index 0ef7dfcc42..c4c2e7a56a 100644 --- a/vector_search/serializers.py +++ b/vector_search/serializers.py @@ -20,10 +20,6 @@ SearchResponseMetadata, SearchResponseSerializer, ) -from vector_search.constants import ( - QDRANT_CONTENT_FILE_PARAM_MAP, - QDRANT_RESOURCE_PARAM_MAP, -) class LearningResourcesVectorSearchRequestSerializer(serializers.Serializer): @@ -39,22 +35,6 @@ class LearningResourcesVectorSearchRequestSerializer(serializers.Serializer): limit = serializers.IntegerField( required=False, help_text="Number of results to return per page" ) - aggregation_choices = [ - (key, key.replace("_", " ").title()) for key in QDRANT_RESOURCE_PARAM_MAP - ] - aggregations = serializers.ListField( - required=False, - child=serializers.ChoiceField(choices=aggregation_choices), - help_text=( - f"aggregations for facet counts \ - \n\n{build_choice_description_list(aggregation_choices)}" - ), - ) - published = serializers.BooleanField( - required=False, - default=True, - help_text="If the resource is published. We default to True unless passed in", - ) readable_id = serializers.CharField( required=False, help_text="The readable id of the resource" ) @@ -197,11 +177,11 @@ def get_results(self, instance): return instance.get("hits", {}) def get_count(self, instance) -> int: - return instance.get("total", {}).get("value", 0) + return instance.get("total", {}).get("value") - def get_metadata(self, instance) -> SearchResponseMetadata: + def get_metadata(self, _) -> SearchResponseMetadata: return { - "aggregations": instance.get("aggregations", {}), + "aggregations": [], "suggest": [], } @@ -218,17 +198,6 @@ class ContentFileVectorSearchRequestSerializer(serializers.Serializer): limit = serializers.IntegerField( required=False, help_text="Number of results to return per page" ) - aggregation_choices = [ - (key, key.replace("_", " ").title()) for key in QDRANT_CONTENT_FILE_PARAM_MAP - ] - aggregations = serializers.ListField( - required=False, - child=serializers.ChoiceField(choices=aggregation_choices), - help_text=( - f"aggregations for facet counts \ - \n\n{build_choice_description_list(aggregation_choices)}" - ), - ) sortby = serializers.ChoiceField( required=False, choices=CONTENT_FILE_SORTBY_OPTIONS, @@ -306,14 +275,14 @@ class ContentFileVectorSearchResponseSerializer(SearchResponseSerializer): """ def get_count(self, instance) -> int: - return instance.get("total", {}).get("value", 0) + return instance["total"]["value"] @extend_schema_field(ContentFileSerializer(many=True)) def get_results(self, instance): - return instance.get("hits", {}) + return instance["hits"] - def get_metadata(self, instance) -> SearchResponseMetadata: + def get_metadata(self, *_) -> SearchResponseMetadata: return { - "aggregations": instance.get("aggregations", {}), + "aggregations": [], "suggest": [], } diff --git a/vector_search/utils.py b/vector_search/utils.py index 0027445da4..d5a50e05f0 100644 --- a/vector_search/utils.py +++ b/vector_search/utils.py @@ -1,4 +1,3 @@ -import asyncio import gc import logging import uuid @@ -33,13 +32,13 @@ ) from main.utils import checksum_for_content from vector_search.constants import ( - COLLECTION_PARAM_MAP, CONTENT_FILES_COLLECTION_NAME, QDRANT_CONTENT_FILE_INDEXES, QDRANT_CONTENT_FILE_PARAM_MAP, QDRANT_LEARNING_RESOURCE_INDEXES, QDRANT_RESOURCE_PARAM_MAP, QDRANT_TOPIC_INDEXES, + QDRANT_TOPICS_PARAM_MAP, RESOURCES_COLLECTION_NAME, TOPICS_COLLECTION_NAME, ) @@ -872,7 +871,7 @@ def process_batch(docs_batch, summaries_list): def _resource_vector_hits(search_result): - hits = [hit.payload.get("readable_id") for hit in search_result] + hits = [hit.payload["readable_id"] for hit in search_result] """ Always lookup learning resources by readable_id for portability in case we load points from external systems @@ -982,74 +981,17 @@ def document_exists(document, collection_name=RESOURCES_COLLECTION_NAME): return count_result.count > 0 -async def async_qdrant_aggregations( - aggregation_keys: list, - params: dict, - collection_name: str = RESOURCES_COLLECTION_NAME, -) -> dict: - """ - Compute facet aggregations from Qdrant for each requested field. - Issues one concurrent facet query per aggregation key and returns results - in the same shape used by the OpenSearch aggregation API: - ``{"delivery": [{"key": "online", "doc_count": 24}, ...], ...}`` - Args: - aggregation_keys: list of aggregation parameter names. - Must be valid keys in the collection's param map - (e.g. ``QDRANT_RESOURCE_PARAM_MAP``). - params: dict of all search parameters, which are used to construct - a Qdrant ``models.Filter`` for each facet query. - collection_name: name of the Qdrant collection to query. - Returns: - dict mapping each requested aggregation name to a list of - ``{"key": str, "doc_count": int}`` dicts sorted by - ``doc_count`` descending. - """ - if not aggregation_keys: - return {} - - param_map = COLLECTION_PARAM_MAP.get(collection_name, QDRANT_RESOURCE_PARAM_MAP) - client = async_qdrant_client() - - async def _get_facet(agg_key: str): - qdrant_field = param_map.get(agg_key) - if not qdrant_field: - return agg_key, [] - - filtered_params = { - k: v for k, v in params.items() if k.partition("__")[0] != agg_key - } - facet_filter = qdrant_query_conditions( - filtered_params, collection_name=collection_name - ) - - result = await client.facet( - collection_name=collection_name, - key=qdrant_field, - facet_filter=facet_filter, - limit=100, - ) - hits = [ - { - "key": str(hit.value).lower() - if isinstance(hit.value, bool) - else str(hit.value), - "doc_count": hit.count, - } - for hit in result.hits - ] - hits.sort(key=lambda x: x["doc_count"], reverse=True) - return agg_key, hits - - results = await asyncio.gather(*[_get_facet(key) for key in aggregation_keys]) - return dict(results) - - def qdrant_query_conditions(params, collection_name=RESOURCES_COLLECTION_NAME): """ Return a list of Qdrant FieldCondition objects based on params """ - qdrant_param_map = COLLECTION_PARAM_MAP.get(collection_name) + collection_param_map = { + RESOURCES_COLLECTION_NAME: QDRANT_RESOURCE_PARAM_MAP, + TOPICS_COLLECTION_NAME: QDRANT_TOPICS_PARAM_MAP, + CONTENT_FILES_COLLECTION_NAME: QDRANT_CONTENT_FILE_PARAM_MAP, + } + qdrant_param_map = collection_param_map.get(collection_name) if not params or not qdrant_param_map: return None must = [] diff --git a/vector_search/utils_test.py b/vector_search/utils_test.py index c9d2458a44..8917834c5b 100644 --- a/vector_search/utils_test.py +++ b/vector_search/utils_test.py @@ -1,4 +1,3 @@ -import asyncio import random from decimal import Decimal from unittest.mock import MagicMock @@ -45,7 +44,6 @@ _get_text_splitter, _is_markdown_content, _resource_vector_hits, - async_qdrant_aggregations, create_qdrant_collections, embed_learning_resources, embed_topics, @@ -1521,247 +1519,3 @@ def test_resource_vector_hits_preserves_qdrant_score_order(): expected_readable_ids = [r.readable_id for r in shuffled] actual_readable_ids = [r["readable_id"] for r in result] assert actual_readable_ids == expected_readable_ids - - -def _make_facet_hit(count=0, value="test"): - """Build a minimal mock that looks like a Qdrant FacetHit.""" - hit = MagicMock() - hit.value = value - hit.count = count - return hit - - -def _make_facet_response(hits): - """Build a minimal mock that looks like a Qdrant FacetResponse.""" - resp = MagicMock() - resp.hits = hits - return resp - - -def test_async_qdrant_aggregations_empty_keys(mocker): - """Should return {} immediately without calling Qdrant when aggregation_keys is empty.""" - mock_client = mocker.AsyncMock() - mocker.patch( - "vector_search.utils.async_qdrant_client", - return_value=mock_client, - ) - result = asyncio.run(async_qdrant_aggregations([], {})) - assert result == {} - mock_client.facet.assert_not_called() - - -def test_async_qdrant_aggregations_unknown_key(mocker): - """An aggregation key not present in the param map should return an empty list.""" - mock_client = mocker.AsyncMock() - mocker.patch( - "vector_search.utils.async_qdrant_client", - return_value=mock_client, - ) - result = asyncio.run( - async_qdrant_aggregations( - ["nonexistent_field"], - {}, - collection_name=RESOURCES_COLLECTION_NAME, - ) - ) - assert result == {"nonexistent_field": []} - mock_client.facet.assert_not_called() - - -def test_async_qdrant_aggregations_single_key(mocker): - """A valid single aggregation key should query Qdrant and return correctly shaped data.""" - mock_client = mocker.AsyncMock() - mocker.patch( - "vector_search.utils.async_qdrant_client", - return_value=mock_client, - ) - - mock_client.facet.return_value = _make_facet_response( - [ - _make_facet_hit(42, value="course"), - _make_facet_hit(7, value="podcast"), - ] - ) - - result = asyncio.run( - async_qdrant_aggregations( - ["resource_type"], - {}, - collection_name=RESOURCES_COLLECTION_NAME, - ) - ) - - assert "resource_type" in result - hits = result["resource_type"] - # Should be sorted descending by doc_count - assert hits[0] == {"key": "course", "doc_count": 42} - assert hits[1] == {"key": "podcast", "doc_count": 7} - - mock_client.facet.assert_awaited_once() - call_kwargs = mock_client.facet.call_args.kwargs - assert call_kwargs["collection_name"] == RESOURCES_COLLECTION_NAME - assert call_kwargs["key"] == QDRANT_RESOURCE_PARAM_MAP["resource_type"] - assert call_kwargs["limit"] == 100 - - -def test_async_qdrant_aggregations_multiple_keys(mocker): - """Multiple valid keys should each issue a concurrent Qdrant facet call.""" - mock_client = mocker.AsyncMock() - mocker.patch( - "vector_search.utils.async_qdrant_client", - return_value=mock_client, - ) - - # Return different data depending on the 'key' kwarg - def _facet_side_effect(**kwargs): - if kwargs["key"] == QDRANT_RESOURCE_PARAM_MAP["resource_type"]: - return _make_facet_response([_make_facet_hit(10, value="course")]) - if kwargs["key"] == QDRANT_RESOURCE_PARAM_MAP["platform"]: - return _make_facet_response( - [_make_facet_hit(30, value="ocw"), _make_facet_hit(20, value="edx")] - ) - return _make_facet_response([]) - - mock_client.facet.side_effect = _facet_side_effect - - result = asyncio.run( - async_qdrant_aggregations( - ["resource_type", "platform"], - {}, - collection_name=RESOURCES_COLLECTION_NAME, - ) - ) - - assert set(result.keys()) == {"resource_type", "platform"} - assert result["resource_type"] == [{"key": "course", "doc_count": 10}] - # Descending sort - assert result["platform"][0] == {"key": "ocw", "doc_count": 30} - assert result["platform"][1] == {"key": "edx", "doc_count": 20} - assert mock_client.facet.await_count == 2 - - -def test_async_qdrant_aggregations_excludes_own_param_from_filter(mocker): - """ - When building the per-facet filter, the aggregation key's own param - must be excluded so that all values for that facet are counted. - """ - mock_client = mocker.AsyncMock() - mocker.patch( - "vector_search.utils.async_qdrant_client", - return_value=mock_client, - ) - mock_client.facet.return_value = _make_facet_response([]) - - params = { - "resource_type": ["course"], - "platform": ["ocw"], - } - - asyncio.run( - async_qdrant_aggregations( - ["resource_type"], - params, - collection_name=RESOURCES_COLLECTION_NAME, - ) - ) - - mock_client.facet.assert_awaited_once() - call_kwargs = mock_client.facet.call_args.kwargs - - # The facet_filter should NOT contain a condition for resource_type - # (it was stripped out so we get all resource_type facet values), - # but it SHOULD still filter by platform. - facet_filter = call_kwargs.get("facet_filter") - # facet_filter is a qdrant models.Filter with must conditions - assert facet_filter is not None - condition_keys = [c.key for c in facet_filter.must if hasattr(c, "key")] - assert QDRANT_RESOURCE_PARAM_MAP["platform"] in condition_keys - assert QDRANT_RESOURCE_PARAM_MAP["resource_type"] not in condition_keys - - -def test_async_qdrant_aggregations_bool_values_lowercased(mocker): - """Boolean hit values must be returned as lowercase strings ('true'/'false').""" - mock_client = mocker.AsyncMock() - mocker.patch( - "vector_search.utils.async_qdrant_client", - return_value=mock_client, - ) - - mock_client.facet.return_value = _make_facet_response( - [ - _make_facet_hit(5, value=True), - _make_facet_hit(3, value=False), - ] - ) - - result = asyncio.run( - async_qdrant_aggregations( - ["free"], - {}, - collection_name=RESOURCES_COLLECTION_NAME, - ) - ) - - keys = {hit["key"] for hit in result["free"]} - assert "true" in keys - assert "false" in keys - # Verify no raw booleans slipped through - assert True not in keys - assert False not in keys - - -def test_async_qdrant_aggregations_sorted_by_doc_count_desc(mocker): - """Results must be sorted by doc_count in descending order.""" - mock_client = mocker.AsyncMock() - mocker.patch( - "vector_search.utils.async_qdrant_client", - return_value=mock_client, - ) - - mock_client.facet.return_value = _make_facet_response( - [ - _make_facet_hit(5, value="edx"), - _make_facet_hit(100, value="ocw"), - _make_facet_hit(20, value="xpro"), - ] - ) - - result = asyncio.run( - async_qdrant_aggregations( - ["platform"], - {}, - collection_name=RESOURCES_COLLECTION_NAME, - ) - ) - - counts = [hit["doc_count"] for hit in result["platform"]] - assert counts == sorted(counts, reverse=True) - - -def test_async_qdrant_aggregations_uses_content_file_param_map(mocker): - """ - When collection_name is CONTENT_FILES_COLLECTION_NAME the function must - use QDRANT_CONTENT_FILE_PARAM_MAP to resolve the Qdrant field name. - """ - mock_client = mocker.AsyncMock() - mocker.patch( - "vector_search.utils.async_qdrant_client", - return_value=mock_client, - ) - mock_client.facet.return_value = _make_facet_response( - [_make_facet_hit(8, value=".pdf")] - ) - - result = asyncio.run( - async_qdrant_aggregations( - ["file_extension"], - {}, - collection_name=CONTENT_FILES_COLLECTION_NAME, - ) - ) - - assert "file_extension" in result - call_kwargs = mock_client.facet.call_args.kwargs - assert call_kwargs["collection_name"] == CONTENT_FILES_COLLECTION_NAME - # The Qdrant field for 'file_extension' should come from the content-file map - assert call_kwargs["key"] == QDRANT_CONTENT_FILE_PARAM_MAP["file_extension"] diff --git a/vector_search/views.py b/vector_search/views.py index 5335351c46..4466edd129 100644 --- a/vector_search/views.py +++ b/vector_search/views.py @@ -17,12 +17,6 @@ from authentication.decorators import blocked_ip_exempt from learning_resources.constants import GROUP_CONTENT_FILE_CONTENT_VIEWERS from main.utils import cache_page_for_anonymous_users -from vector_search.constants import ( - CONTENT_FILES_COLLECTION_NAME, - CONTENT_FILES_RETRIEVE_PAYLOAD, - RESOURCES_COLLECTION_NAME, - RESOURCES_RETRIEVE_PAYLOAD, -) from vector_search.serializers import ( ContentFileVectorSearchRequestSerializer, ContentFileVectorSearchResponseSerializer, @@ -30,10 +24,11 @@ LearningResourcesVectorSearchResponseSerializer, ) from vector_search.utils import ( + CONTENT_FILES_COLLECTION_NAME, + RESOURCES_COLLECTION_NAME, _content_file_vector_hits, _merge_dicts, _resource_vector_hits, - async_qdrant_aggregations, async_qdrant_client, dense_encoder, qdrant_query_conditions, @@ -87,12 +82,12 @@ async def dispatch(self, request, *args, **kwargs): self.response = self.finalize_response(request, response, *args, **kwargs) return self.response - async def async_vector_search( # noqa: PLR0913, PLR0915 + async def async_vector_search( # noqa: PLR0913 self, query_string: str, params: dict, limit: int = 10, - offset: int = 0, + offset: int = 10, search_collection=RESOURCES_COLLECTION_NAME, *, hybrid_search: bool = False, @@ -118,19 +113,8 @@ async def async_vector_search( # noqa: PLR0913, PLR0915 "collection_name": search_collection, "query_filter": search_filter, "with_vectors": False, - "with_payload": RESOURCES_RETRIEVE_PAYLOAD - if search_collection == RESOURCES_COLLECTION_NAME - else CONTENT_FILES_RETRIEVE_PAYLOAD, - "search_params": models.SearchParams( - quantization=models.QuantizationSearchParams( - ignore=False, - rescore=True, - oversampling=1, - ), - hnsw_ef=64, - indexed_only=True, - exact=False, - ), + "with_payload": True, + "search_params": models.SearchParams(indexed_only=True, exact=False), "limit": limit, } @@ -167,7 +151,6 @@ async def async_vector_search( # noqa: PLR0913, PLR0915 search_params.pop("search_params", None) search_params["group_by"] = params.get("group_by") search_params["group_size"] = params.get("group_size", 1) - search_params["with_payload"] = True group_result = await client.query_points_groups(**search_params) search_result = [] for group in group_result.groups: @@ -188,56 +171,29 @@ async def async_vector_search( # noqa: PLR0913, PLR0915 result_obj = await client.query_points(**search_params) search_result = result_obj.points else: - # Qdrant's scroll API uses a point-ID cursor for `offset`, not a - # numeric skip count. We implement integer offset by consuming - # scroll pages until the desired number of records are skipped. - remaining_to_skip = offset - next_page_offset = None - search_result = [] - while True: - fetch_size = min(max(remaining_to_skip, limit), 1000) - scroll_res = await client.scroll( - collection_name=search_collection, - scroll_filter=search_filter, - limit=fetch_size, - offset=next_page_offset, - with_vectors=False, - ) - page_points, next_page_offset = scroll_res - if remaining_to_skip > 0: - skipped = min(remaining_to_skip, len(page_points)) - page_points = page_points[skipped:] - remaining_to_skip -= skipped - search_result.extend(page_points) - if len(search_result) >= limit or not next_page_offset: - break - search_result = search_result[:limit] - - hits_coroutine = ( - sync_to_async(_resource_vector_hits)(search_result) - if search_collection == RESOURCES_COLLECTION_NAME - else sync_to_async(_content_file_vector_hits)(search_result) - ) - - aggregation_keys = params.get("aggregations") or [] - hits, count_result, aggregations = await asyncio.gather( - hits_coroutine, - client.count( - collection_name=search_collection, - count_filter=search_filter, - exact=False, - ), - async_qdrant_aggregations( - aggregation_keys, - params, + scroll_res = await client.scroll( collection_name=search_collection, - ), + scroll_filter=search_filter, + limit=limit, + offset=offset, + with_vectors=False, + ) + search_result = scroll_res[0] + + if search_collection == RESOURCES_COLLECTION_NAME: + hits = await sync_to_async(_resource_vector_hits)(search_result) + else: + hits = await sync_to_async(_content_file_vector_hits)(search_result) + + count_result = await client.count( + collection_name=search_collection, + count_filter=search_filter, + exact=False, ) return { "hits": hits, "total": {"value": count_result.count}, - "aggregations": aggregations or {}, } def handle_exception(self, exc): diff --git a/vector_search/views_test.py b/vector_search/views_test.py index 981fc4371b..774ca25436 100644 --- a/vector_search/views_test.py +++ b/vector_search/views_test.py @@ -14,7 +14,7 @@ def test_vector_search_filters(mocker, client): mock_qdrant = mocker.patch( "qdrant_client.AsyncQdrantClient", return_value=mocker.AsyncMock() )() - mock_qdrant.scroll = mocker.AsyncMock(return_value=([], None)) + mock_qdrant.scroll = mocker.AsyncMock(return_value=[[]]) mock_qdrant.query_points = mocker.AsyncMock() mock_qdrant.query_points_groups = mocker.AsyncMock() mocker.patch( @@ -63,7 +63,7 @@ def test_vector_search_filters_empty_query(mocker, client): mock_qdrant = mocker.patch( "qdrant_client.AsyncQdrantClient", return_value=mocker.AsyncMock() )() - mock_qdrant.scroll = mocker.AsyncMock(return_value=([], None)) + mock_qdrant.scroll = mocker.AsyncMock(return_value=[[]]) mock_qdrant.query_points = mocker.AsyncMock() mock_qdrant.query_points_groups = mocker.AsyncMock() mock_qdrant.count = mocker.AsyncMock(return_value=CountResult(count=10)) @@ -124,7 +124,7 @@ def test_content_file_vector_search_filters( mock_qdrant = mocker.patch( "qdrant_client.AsyncQdrantClient", return_value=mocker.AsyncMock() )() - mock_qdrant.scroll = mocker.AsyncMock(return_value=([], None)) + mock_qdrant.scroll = mocker.AsyncMock(return_value=[[]]) mock_qdrant.query_points = mocker.AsyncMock() mock_qdrant.query_points_groups = mocker.AsyncMock() mocker.patch( @@ -201,7 +201,7 @@ def test_content_file_vector_search_filters_empty_query( mock_qdrant = mocker.patch( "qdrant_client.AsyncQdrantClient", return_value=mocker.AsyncMock() )() - mock_qdrant.scroll = mocker.AsyncMock(return_value=([], None)) + mock_qdrant.scroll = mocker.AsyncMock(return_value=[[]]) mock_qdrant.query_points = mocker.AsyncMock() mock_qdrant.query_points_groups = mocker.AsyncMock() mocker.patch( @@ -255,7 +255,7 @@ def test_content_file_vector_search_filters_custom_collection( "qdrant_client.AsyncQdrantClient", return_value=mocker.AsyncMock() )() custom_collection_name = "foo_bar_collection" - mock_qdrant.scroll = mocker.AsyncMock(return_value=([], None)) + mock_qdrant.scroll = mocker.AsyncMock(return_value=[[]]) mock_qdrant.query_points = mocker.AsyncMock() mock_qdrant.query_points_groups = mocker.AsyncMock() mocker.patch( @@ -300,7 +300,7 @@ def test_content_file_vector_search_group_parameters(mocker, client, django_user )() custom_collection_name = "foo_bar_collection" - mock_qdrant.scroll = mocker.AsyncMock(return_value=([], None)) + mock_qdrant.scroll = mocker.AsyncMock(return_value=[[]]) mock_qdrant.query_points = mocker.AsyncMock() mock_qdrant.query_points_groups = mocker.AsyncMock() mocker.patch( From 45ecb5010373bd1b72e90dd03c1b9b55a037c356 Mon Sep 17 00:00:00 2001 From: Doof Date: Wed, 15 Apr 2026 19:30:28 +0000 Subject: [PATCH 5/5] Release 0.63.5 --- RELEASE.rst | 8 ++++++++ main/settings.py | 2 +- 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/RELEASE.rst b/RELEASE.rst index 3eb0c8b68a..e7bda78973 100644 --- a/RELEASE.rst +++ b/RELEASE.rst @@ -1,6 +1,14 @@ Release Notes ============= +Version 0.63.5 +-------------- + +- Revert "Facet counts and aggregations for Vector search (#3188)" (#3208) +- hide stay updated button on free products (#3201) +- Deindex unpublished test_mode resources, not contentfiles (#3192) +- feat: enable course access to staff user before the start date (#3182) + Version 0.63.3 (Released April 15, 2026) -------------- diff --git a/main/settings.py b/main/settings.py index ca08d99998..9f01c3535f 100644 --- a/main/settings.py +++ b/main/settings.py @@ -34,7 +34,7 @@ from main.settings_pluggy import * # noqa: F403 from openapi.settings_spectacular import open_spectacular_settings -VERSION = "0.63.3" +VERSION = "0.63.5" log = logging.getLogger()