From 15423c5977aae25ea8cce45939987a14936008c6 Mon Sep 17 00:00:00 2001 From: Shankar Ambady Date: Thu, 23 Apr 2026 11:43:31 -0400 Subject: [PATCH 1/5] vector learning resources sortby/sorting support (#3228) * switching to field names for sort params * add descending keys to sort param * refactor/breakup vector_search method. integrate formatting of sort parameter * dynamic sortby fields * remove sortby from contentfile vector endpoint * adding frontend changes * add test for format_order_by * adding test for sortby parameter * fix param formatting * remove typecast --- frontends/api/src/generated/v0/api.ts | 71 ++--- .../SearchDisplay/SearchDisplay.tsx | 19 +- openapi/specs/v0.yaml | 47 +-- vector_search/constants.py | 33 +++ vector_search/serializers.py | 14 +- vector_search/views.py | 279 ++++++++++++------ vector_search/views_test.py | 55 ++++ 7 files changed, 364 insertions(+), 154 deletions(-) diff --git a/frontends/api/src/generated/v0/api.ts b/frontends/api/src/generated/v0/api.ts index 46c5b27485..ccc0941c53 100644 --- a/frontends/api/src/generated/v0/api.ts +++ b/frontends/api/src/generated/v0/api.ts @@ -11552,7 +11552,6 @@ export const VectorContentFilesSearchApiAxiosParamCreator = function ( * @param {string} [q] The search text * @param {Array} [resource_readable_id] The readable_id value of the parent learning resource for the content file * @param {Array} [run_readable_id] The readable_id value of the run that the content file belongs to - * @param {VectorContentFilesSearchRetrieveSortbyEnum} [sortby] if the parameter starts with \'-\' the sort is in descending order * `id` - id * `-id` - -id * `resource_readable_id` - resource_readable_id * `-resource_readable_id` - -resource_readable_id * @param {boolean | null} [title__isnull] Filter to content files where title is null/not null * @param {boolean | null} [url__isnull] Filter to content files where url is null/not null * @param {*} [options] Override http request option. @@ -11573,7 +11572,6 @@ export const VectorContentFilesSearchApiAxiosParamCreator = function ( q?: string, resource_readable_id?: Array, run_readable_id?: Array, - sortby?: VectorContentFilesSearchRetrieveSortbyEnum, title__isnull?: boolean | null, url__isnull?: boolean | null, options: RawAxiosRequestConfig = {}, @@ -11650,10 +11648,6 @@ export const VectorContentFilesSearchApiAxiosParamCreator = function ( localVarQueryParameter["run_readable_id"] = run_readable_id } - if (sortby !== undefined) { - localVarQueryParameter["sortby"] = sortby - } - if (title__isnull !== undefined) { localVarQueryParameter["title__isnull"] = title__isnull } @@ -11706,7 +11700,6 @@ export const VectorContentFilesSearchApiFp = function ( * @param {string} [q] The search text * @param {Array} [resource_readable_id] The readable_id value of the parent learning resource for the content file * @param {Array} [run_readable_id] The readable_id value of the run that the content file belongs to - * @param {VectorContentFilesSearchRetrieveSortbyEnum} [sortby] if the parameter starts with \'-\' the sort is in descending order * `id` - id * `-id` - -id * `resource_readable_id` - resource_readable_id * `-resource_readable_id` - -resource_readable_id * @param {boolean | null} [title__isnull] Filter to content files where title is null/not null * @param {boolean | null} [url__isnull] Filter to content files where url is null/not null * @param {*} [options] Override http request option. @@ -11727,7 +11720,6 @@ export const VectorContentFilesSearchApiFp = function ( q?: string, resource_readable_id?: Array, run_readable_id?: Array, - sortby?: VectorContentFilesSearchRetrieveSortbyEnum, title__isnull?: boolean | null, url__isnull?: boolean | null, options?: RawAxiosRequestConfig, @@ -11753,7 +11745,6 @@ export const VectorContentFilesSearchApiFp = function ( q, resource_readable_id, run_readable_id, - sortby, title__isnull, url__isnull, options, @@ -11812,7 +11803,6 @@ export const VectorContentFilesSearchApiFactory = function ( requestParameters.q, requestParameters.resource_readable_id, requestParameters.run_readable_id, - requestParameters.sortby, requestParameters.title__isnull, requestParameters.url__isnull, options, @@ -11926,13 +11916,6 @@ export interface VectorContentFilesSearchApiVectorContentFilesSearchRetrieveRequ */ readonly run_readable_id?: Array - /** - * if the parameter starts with \'-\' the sort is in descending order * `id` - id * `-id` - -id * `resource_readable_id` - resource_readable_id * `-resource_readable_id` - -resource_readable_id - * @type {'id' | '-id' | 'resource_readable_id' | '-resource_readable_id'} - * @memberof VectorContentFilesSearchApiVectorContentFilesSearchRetrieve - */ - readonly sortby?: VectorContentFilesSearchRetrieveSortbyEnum - /** * Filter to content files where title is null/not null * @type {boolean} @@ -11983,7 +11966,6 @@ export class VectorContentFilesSearchApi extends BaseAPI { requestParameters.q, requestParameters.resource_readable_id, requestParameters.run_readable_id, - requestParameters.sortby, requestParameters.title__isnull, requestParameters.url__isnull, options, @@ -12017,17 +11999,6 @@ export const VectorContentFilesSearchRetrieveAggregationsEnum = { } as const export type VectorContentFilesSearchRetrieveAggregationsEnum = (typeof VectorContentFilesSearchRetrieveAggregationsEnum)[keyof typeof VectorContentFilesSearchRetrieveAggregationsEnum] -/** - * @export - */ -export const VectorContentFilesSearchRetrieveSortbyEnum = { - Id: "id", - Id2: "-id", - ResourceReadableId: "resource_readable_id", - ResourceReadableId2: "-resource_readable_id", -} as const -export type VectorContentFilesSearchRetrieveSortbyEnum = - (typeof VectorContentFilesSearchRetrieveSortbyEnum)[keyof typeof VectorContentFilesSearchRetrieveSortbyEnum] /** * VectorLearningResourcesSearchApi - axios parameter creator @@ -12040,7 +12011,7 @@ 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 {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 * `next_start_date` - Next Start Date * `views` - Views * `created_on` - Created On * @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/ @@ -12060,6 +12031,7 @@ export const VectorLearningResourcesSearchApiAxiosParamCreator = function ( * @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 * @param {Array} [resource_type_group] The category of learning resource * `course` - Course * `program` - Program * `learning_material` - Learning Material + * @param {VectorLearningResourcesSearchRetrieveSortbyEnum} [sortby] if the parameter starts with \'-\' the sort is in descending order * `next_start_date` - next_start_date * `views` - views * `created_on` - created_on * `-next_start_date` - -next_start_date * `-views` - -views * `-created_on` - -created_on * @param {boolean | null} [title__isnull] Filter to learning resources where title is null/not null * @param {Array} [topic] The topic name. To see a list of options go to api/v1/topics/ * @param {boolean | null} [url__isnull] Filter to learning resources where url is null/not null @@ -12087,6 +12059,7 @@ export const VectorLearningResourcesSearchApiAxiosParamCreator = function ( readable_id?: string, resource_type?: Array, resource_type_group?: Array, + sortby?: VectorLearningResourcesSearchRetrieveSortbyEnum, title__isnull?: boolean | null, topic?: Array, url__isnull?: boolean | null, @@ -12188,6 +12161,10 @@ export const VectorLearningResourcesSearchApiAxiosParamCreator = function ( localVarQueryParameter["resource_type_group"] = resource_type_group } + if (sortby !== undefined) { + localVarQueryParameter["sortby"] = sortby + } + if (title__isnull !== undefined) { localVarQueryParameter["title__isnull"] = title__isnull } @@ -12230,7 +12207,7 @@ 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 {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 * `next_start_date` - Next Start Date * `views` - Views * `created_on` - Created On * @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/ @@ -12250,6 +12227,7 @@ export const VectorLearningResourcesSearchApiFp = function ( * @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 * @param {Array} [resource_type_group] The category of learning resource * `course` - Course * `program` - Program * `learning_material` - Learning Material + * @param {VectorLearningResourcesSearchRetrieveSortbyEnum} [sortby] if the parameter starts with \'-\' the sort is in descending order * `next_start_date` - next_start_date * `views` - views * `created_on` - created_on * `-next_start_date` - -next_start_date * `-views` - -views * `-created_on` - -created_on * @param {boolean | null} [title__isnull] Filter to learning resources where title is null/not null * @param {Array} [topic] The topic name. To see a list of options go to api/v1/topics/ * @param {boolean | null} [url__isnull] Filter to learning resources where url is null/not null @@ -12277,6 +12255,7 @@ export const VectorLearningResourcesSearchApiFp = function ( readable_id?: string, resource_type?: Array, resource_type_group?: Array, + sortby?: VectorLearningResourcesSearchRetrieveSortbyEnum, title__isnull?: boolean | null, topic?: Array, url__isnull?: boolean | null, @@ -12309,6 +12288,7 @@ export const VectorLearningResourcesSearchApiFp = function ( readable_id, resource_type, resource_type_group, + sortby, title__isnull, topic, url__isnull, @@ -12374,6 +12354,7 @@ export const VectorLearningResourcesSearchApiFactory = function ( requestParameters.readable_id, requestParameters.resource_type, requestParameters.resource_type_group, + requestParameters.sortby, requestParameters.title__isnull, requestParameters.topic, requestParameters.url__isnull, @@ -12391,8 +12372,8 @@ export const VectorLearningResourcesSearchApiFactory = function ( */ 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'>} + * 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 * `next_start_date` - Next Start Date * `views` - Views * `created_on` - Created On + * @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' | 'next_start_date' | 'views' | 'created_on'>} * @memberof VectorLearningResourcesSearchApiVectorLearningResourcesSearchRetrieve */ readonly aggregations?: Array @@ -12530,6 +12511,13 @@ export interface VectorLearningResourcesSearchApiVectorLearningResourcesSearchRe */ readonly resource_type_group?: Array + /** + * if the parameter starts with \'-\' the sort is in descending order * `next_start_date` - next_start_date * `views` - views * `created_on` - created_on * `-next_start_date` - -next_start_date * `-views` - -views * `-created_on` - -created_on + * @type {'next_start_date' | 'views' | 'created_on' | '-next_start_date' | '-views' | '-created_on'} + * @memberof VectorLearningResourcesSearchApiVectorLearningResourcesSearchRetrieve + */ + readonly sortby?: VectorLearningResourcesSearchRetrieveSortbyEnum + /** * Filter to learning resources where title is null/not null * @type {boolean} @@ -12593,6 +12581,7 @@ export class VectorLearningResourcesSearchApi extends BaseAPI { requestParameters.readable_id, requestParameters.resource_type, requestParameters.resource_type_group, + requestParameters.sortby, requestParameters.title__isnull, requestParameters.topic, requestParameters.url__isnull, @@ -12625,6 +12614,9 @@ export const VectorLearningResourcesSearchRetrieveAggregationsEnum = { ResourceTypeGroup: "resource_type_group", ResourceCategory: "resource_category", Published: "published", + NextStartDate: "next_start_date", + Views: "views", + CreatedOn: "created_on", } as const export type VectorLearningResourcesSearchRetrieveAggregationsEnum = (typeof VectorLearningResourcesSearchRetrieveAggregationsEnum)[keyof typeof VectorLearningResourcesSearchRetrieveAggregationsEnum] @@ -12775,6 +12767,19 @@ export const VectorLearningResourcesSearchRetrieveResourceTypeGroupEnum = { } as const export type VectorLearningResourcesSearchRetrieveResourceTypeGroupEnum = (typeof VectorLearningResourcesSearchRetrieveResourceTypeGroupEnum)[keyof typeof VectorLearningResourcesSearchRetrieveResourceTypeGroupEnum] +/** + * @export + */ +export const VectorLearningResourcesSearchRetrieveSortbyEnum = { + NextStartDate: "next_start_date", + Views: "views", + CreatedOn: "created_on", + NextStartDate2: "-next_start_date", + Views2: "-views", + CreatedOn2: "-created_on", +} as const +export type VectorLearningResourcesSearchRetrieveSortbyEnum = + (typeof VectorLearningResourcesSearchRetrieveSortbyEnum)[keyof typeof VectorLearningResourcesSearchRetrieveSortbyEnum] /** * VideoShortsApi - axios parameter creator diff --git a/frontends/main/src/page-components/SearchDisplay/SearchDisplay.tsx b/frontends/main/src/page-components/SearchDisplay/SearchDisplay.tsx index e0b53a8ee8..c6c8cb2e27 100644 --- a/frontends/main/src/page-components/SearchDisplay/SearchDisplay.tsx +++ b/frontends/main/src/page-components/SearchDisplay/SearchDisplay.tsx @@ -514,6 +514,22 @@ const searchModeDropdownOptions = Object.entries( SearchModeEnumDescriptions, ).map(([label, value]) => ({ label, value })) +const mapVectorSortby = ( + sortby?: string, +): VectorSearchRequest["sortby"] | undefined => { + switch (sortby) { + case "-views": + case "popular": + return "-views" + case "upcoming": + return "next_start_date" + case "new": + return "-created_on" + default: + return undefined + } +} + /** * 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) @@ -524,7 +540,7 @@ const searchModeDropdownOptions = Object.entries( * string-literal values (e.g., delivery: 'online' | 'hybrid' | ...). */ const toVectorSearchParams = ( - params: ReturnType, + params: ReturnType & { sortby?: string }, ): VectorSearchRequest => ({ aggregations: params.aggregations as VectorSearchRequest["aggregations"], certification: params.certification, @@ -545,6 +561,7 @@ const toVectorSearchParams = ( resource_type: params.resource_type as VectorSearchRequest["resource_type"], resource_type_group: params.resource_type_group as VectorSearchRequest["resource_type_group"], + sortby: mapVectorSortby(params.sortby), topic: params.topic, hybrid_search: true, }) diff --git a/openapi/specs/v0.yaml b/openapi/specs/v0.yaml index a75deb2939..2a65b6cd16 100644 --- a/openapi/specs/v0.yaml +++ b/openapi/specs/v0.yaml @@ -969,23 +969,6 @@ paths: minLength: 1 description: The readable_id value of the run that the content file belongs to - - in: query - name: sortby - schema: - enum: - - id - - -id - - resource_readable_id - - -resource_readable_id - type: string - minLength: 1 - description: |- - if the parameter starts with '-' the sort is in descending order - - * `id` - id - * `-id` - -id - * `resource_readable_id` - resource_readable_id - * `-resource_readable_id` - -resource_readable_id - in: query name: title__isnull schema: @@ -1038,6 +1021,9 @@ paths: - resource_type_group - resource_category - published + - next_start_date + - views + - created_on type: string description: |- * `readable_id` - Readable Id @@ -1059,6 +1045,9 @@ paths: * `resource_type_group` - Resource Type Group * `resource_category` - Resource Category * `published` - Published + * `next_start_date` - Next Start Date + * `views` - Views + * `created_on` - Created On 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`\ @@ -1067,7 +1056,8 @@ paths: \ - 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" + \ - Published\n* `next_start_date` - Next Start Date\n* `views` - Views\n\ + * `created_on` - Created On" - in: query name: certification schema: @@ -1425,6 +1415,27 @@ paths: * `learning_material` - Learning Material description: "The category of learning resource \n\n* `course`\ \ - Course\n* `program` - Program\n* `learning_material` - Learning Material" + - in: query + name: sortby + schema: + enum: + - next_start_date + - views + - created_on + - -next_start_date + - -views + - -created_on + type: string + minLength: 1 + description: |- + if the parameter starts with '-' the sort is in descending order + + * `next_start_date` - next_start_date + * `views` - views + * `created_on` - created_on + * `-next_start_date` - -next_start_date + * `-views` - -views + * `-created_on` - -created_on - in: query name: title__isnull schema: diff --git a/vector_search/constants.py b/vector_search/constants.py index 074fa67991..5aa7223a57 100644 --- a/vector_search/constants.py +++ b/vector_search/constants.py @@ -47,6 +47,9 @@ "resource_type_group": "resource_type_group", "resource_category": "resource_category", "published": "published", + "next_start_date": "next_start_date", + "views": "views", + "created_on": "created_on", } @@ -74,8 +77,24 @@ "title": models.PayloadSchemaType.KEYWORD, "resource_type_group": models.PayloadSchemaType.KEYWORD, "resource_category": models.PayloadSchemaType.KEYWORD, + "next_start_date": models.PayloadSchemaType.DATETIME, + "created_on": models.PayloadSchemaType.DATETIME, + "views": models.PayloadSchemaType.INTEGER, } + +QDRANT_LEARNING_RESOURCE_SORTBY_FIELDS = [ + param + for param in QDRANT_RESOURCE_PARAM_MAP + if QDRANT_RESOURCE_PARAM_MAP[param] in QDRANT_LEARNING_RESOURCE_INDEXES + and QDRANT_LEARNING_RESOURCE_INDEXES[QDRANT_RESOURCE_PARAM_MAP[param]] + in [ + models.PayloadSchemaType.DATETIME, + models.PayloadSchemaType.INTEGER, + models.PayloadSchemaType.FLOAT, + models.PayloadSchemaType.UUID, + ] +] """ Note: Be intentional about which fields we add as indexes. Only add fields that we expect to filter or facet on frequently. @@ -92,6 +111,20 @@ "url": models.PayloadSchemaType.KEYWORD, } + +QDRANT_CONTENT_FILES_SORTBY_FIELDS = [ + param + for param in QDRANT_CONTENT_FILE_PARAM_MAP + if QDRANT_CONTENT_FILE_PARAM_MAP[param] in QDRANT_CONTENT_FILE_INDEXES + and QDRANT_CONTENT_FILE_INDEXES[QDRANT_CONTENT_FILE_PARAM_MAP[param]] + in [ + models.PayloadSchemaType.DATETIME, + models.PayloadSchemaType.INTEGER, + models.PayloadSchemaType.FLOAT, + models.PayloadSchemaType.UUID, + ] +] + QDRANT_TOPIC_INDEXES = { "name": models.PayloadSchemaType.KEYWORD, } diff --git a/vector_search/serializers.py b/vector_search/serializers.py index dbd5614467..47aee71862 100644 --- a/vector_search/serializers.py +++ b/vector_search/serializers.py @@ -14,7 +14,6 @@ ) from learning_resources.serializers import LearningResourceSerializer from learning_resources_search.serializers import ( - CONTENT_FILE_SORTBY_OPTIONS, ArrayWrappedBoolean, ContentFileSerializer, SearchResponseMetadata, @@ -22,6 +21,7 @@ ) from vector_search.constants import ( QDRANT_CONTENT_FILE_PARAM_MAP, + QDRANT_LEARNING_RESOURCE_SORTBY_FIELDS, QDRANT_RESOURCE_PARAM_MAP, ) @@ -160,6 +160,12 @@ class LearningResourcesVectorSearchRequestSerializer( default=True, help_text="If the resource is published. We default to True unless passed in", ) + sortby = serializers.ChoiceField( + required=False, + choices=QDRANT_LEARNING_RESOURCE_SORTBY_FIELDS + + [f"-{param}" for param in QDRANT_LEARNING_RESOURCE_SORTBY_FIELDS], + help_text="if the parameter starts with '-' the sort is in descending order", + ) aggregation_choices = [ (key, key.replace("_", " ").title()) for key in QDRANT_RESOURCE_PARAM_MAP ] @@ -243,11 +249,7 @@ class ContentFileVectorSearchRequestSerializer(serializers.Serializer): \n\n{build_choice_description_list(aggregation_choices)}" ), ) - sortby = serializers.ChoiceField( - required=False, - choices=CONTENT_FILE_SORTBY_OPTIONS, - help_text="if the parameter starts with '-' the sort is in descending order", - ) + key = serializers.ListField( required=False, child=serializers.CharField(), diff --git a/vector_search/views.py b/vector_search/views.py index 5335351c46..8031f599b8 100644 --- a/vector_search/views.py +++ b/vector_search/views.py @@ -87,62 +87,76 @@ 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 + def _format_order_by(self, order_by_parameter): + sort = models.Direction.ASC + if order_by_parameter.startswith("-") and len(order_by_parameter) > 1: + order_by_parameter = order_by_parameter.lstrip("-") + sort = models.Direction.DESC + return models.OrderBy(key=order_by_parameter, direction=sort) + + async def _build_search_params( # noqa: PLR0913 self, query_string: str, - params: dict, - limit: int = 10, - offset: int = 0, - search_collection=RESOURCES_COLLECTION_NAME, - *, - hybrid_search: bool = False, + search_collection: str, + search_filter, + limit: int, + prefetch_limit: int, + order_by: str, + encoder_dense, + encoder_sparse, + hybrid_search, ): - client = async_qdrant_client() - encoder_dense = dense_encoder() - encoder_sparse = sparse_encoder() - - search_filter = qdrant_query_conditions( - params, collection_name=search_collection - ) - prefetch_multiplier = getattr( - settings, "VECTOR_HYBRID_SEARCH_PREFETCH_MULTIPLIER", 20 - ) - prefetch_limit = (offset + limit) * prefetch_multiplier - prefetch_max_limit = getattr( - settings, "VECTOR_HYBRID_SEARCH_PREFETCH_MAX_LIMIT", None - ) - if prefetch_max_limit is not None: - prefetch_limit = min(prefetch_limit, prefetch_max_limit) - if query_string: - search_params = { - "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, + search_params = { + "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, ), - "limit": limit, - } + hnsw_ef=64, + indexed_only=True, + exact=False, + ), + "limit": limit, + } - if hybrid_search: - sparse_query, dense_query = await asyncio.gather( - sync_to_async(encoder_sparse.embed, thread_sensitive=False)( - query_string - ), - sync_to_async(encoder_dense.embed_query, thread_sensitive=False)( - query_string - ), + if hybrid_search: + sparse_query, dense_query = await asyncio.gather( + sync_to_async(encoder_sparse.embed, thread_sensitive=False)( + query_string + ), + sync_to_async(encoder_dense.embed_query, thread_sensitive=False)( + query_string + ), + ) + if order_by: + # Nest: vector prefetches → fusion prefetch → order_by query + search_params["prefetch"] = models.Prefetch( + prefetch=[ + models.Prefetch( + query=sparse_query, + using=encoder_sparse.model_short_name(), + limit=prefetch_limit, + ), + models.Prefetch( + query=dense_query, + using=encoder_dense.model_short_name(), + limit=prefetch_limit, + ), + ], + query=models.FusionQuery(fusion=models.Fusion.RRF), + limit=prefetch_limit, + ) + search_params["query"] = models.OrderByQuery( + order_by=self._format_order_by(order_by) ) + else: search_params["prefetch"] = [ models.Prefetch( query=sparse_query, @@ -156,62 +170,133 @@ async def async_vector_search( # noqa: PLR0913, PLR0915 ), ] search_params["query"] = models.FusionQuery(fusion=models.Fusion.RRF) - else: - dense_query = await sync_to_async(encoder_dense.embed_query)( - query_string + else: + dense_query = await sync_to_async(encoder_dense.embed_query)(query_string) + if order_by: + # Nest: dense vector prefetch → order_by query + search_params["prefetch"] = models.Prefetch( + query=dense_query, + using=encoder_dense.model_short_name(), + limit=prefetch_limit, ) + search_params["query"] = models.OrderByQuery( + order_by=self._format_order_by(order_by) + ) + else: search_params["using"] = encoder_dense.model_short_name() search_params["query"] = dense_query - if "group_by" in params: - 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: - payloads = [hit.payload for hit in group.hits] - response_hit = _merge_dicts(payloads) - chunks = [payload.get("chunk_content") for payload in payloads] - response_hit["chunk_content"] = None - response_hit["chunks"] = chunks - response_row = { - "id": response_hit[search_params["group_by"]], - "payload": response_hit, - "vector": [], - } - search_result.append(models.PointStruct(**response_row)) + return search_params + + async def _execute_group_search(self, client, search_params, params): + 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: + payloads = [hit.payload for hit in group.hits] + response_hit = _merge_dicts(payloads) + chunks = [payload.get("chunk_content") for payload in payloads] + response_hit["chunk_content"] = None + response_hit["chunks"] = chunks + response_row = { + "id": response_hit[search_params["group_by"]], + "payload": response_hit, + "vector": [], + } + search_result.append(models.PointStruct(**response_row)) + return search_result + async def _execute_scroll_search( # noqa: PLR0913 + self, client, search_collection, search_filter, limit, offset, order_by + ): + remaining_to_skip = offset + next_page_offset = None + search_result = [] + + # Build common scroll kwargs + scroll_kwargs = { + "collection_name": search_collection, + "scroll_filter": search_filter, + "with_vectors": False, + } + if order_by: + scroll_kwargs["order_by"] = self._format_order_by(order_by) + + while True: + fetch_size = min(max(remaining_to_skip, limit), 1000) + scroll_res = await client.scroll( + **scroll_kwargs, + limit=fetch_size, + offset=next_page_offset, + ) + 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 + return search_result[:limit] + + async def async_vector_search( # noqa: PLR0913 + self, + query_string: str, + params: dict, + order_by: str | None = None, + limit: int = 10, + offset: int = 0, + search_collection=RESOURCES_COLLECTION_NAME, + *, + hybrid_search: bool = False, + ): + client = async_qdrant_client() + encoder_dense = dense_encoder() + encoder_sparse = sparse_encoder() + + search_filter = qdrant_query_conditions( + params, collection_name=search_collection + ) + + if query_string: + prefetch_multiplier = getattr( + settings, "VECTOR_HYBRID_SEARCH_PREFETCH_MULTIPLIER", 20 + ) + prefetch_limit = (offset + limit) * prefetch_multiplier + prefetch_max_limit = getattr( + settings, "VECTOR_HYBRID_SEARCH_PREFETCH_MAX_LIMIT", None + ) + if prefetch_max_limit is not None: + prefetch_limit = min(prefetch_limit, prefetch_max_limit) + + search_params = await self._build_search_params( + query_string, + search_collection, + search_filter, + limit, + prefetch_limit, + order_by, + encoder_dense, + encoder_sparse, + hybrid_search, + ) + + if "group_by" in params: + search_result = await self._execute_group_search( + client, search_params, params + ) else: search_params["offset"] = offset 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] + # No query string — use scroll API + search_result = await self._execute_scroll_search( + client, search_collection, search_filter, limit, offset, order_by + ) hits_coroutine = ( sync_to_async(_resource_vector_hits)(search_result) @@ -280,8 +365,10 @@ async def get(self, request): hybrid_search = request_data.data.get("hybrid_search", False) limit = request_data.data.get("limit", 10) offset = request_data.data.get("offset", 0) + order_by = request_data.data.get("sortby") response = await self.async_vector_search( query_text, + order_by=order_by, limit=limit, offset=offset, params=request_data.data, diff --git a/vector_search/views_test.py b/vector_search/views_test.py index 981fc4371b..a166362b7e 100644 --- a/vector_search/views_test.py +++ b/vector_search/views_test.py @@ -6,6 +6,7 @@ from rest_framework.exceptions import NotAuthenticated, PermissionDenied from learning_resources.constants import GROUP_CONTENT_FILE_CONTENT_VIEWERS +from vector_search.views import QdrantView def test_vector_search_filters(mocker, client): @@ -366,3 +367,57 @@ def test_content_file_vector_search_group_parameters(mocker, client, django_user .kwargs["collection_name"] .endswith(custom_collection_name) ) + + +def test_qdrant_view_format_order_by(): + view = QdrantView() + + order_by = view._format_order_by("views") # noqa: SLF001 + assert order_by.key == "views" + assert order_by.direction == models.Direction.ASC + + order_by = view._format_order_by("-views") # noqa: SLF001 + assert order_by.key == "views" + assert order_by.direction == models.Direction.DESC + + order_by = view._format_order_by("-") # noqa: SLF001 + assert order_by.key == "-" + assert order_by.direction == models.Direction.ASC + + +@pytest.mark.parametrize("query_string", ["", "test"]) +@pytest.mark.parametrize("hybrid_search", [True, False]) +def test_vector_search_sortby_parameter(mocker, client, query_string, hybrid_search): + """Test vector search properly passes sortby parameter to qdrant client""" + + mock_qdrant = mocker.patch( + "qdrant_client.AsyncQdrantClient", return_value=mocker.AsyncMock() + )() + mock_qdrant.scroll = mocker.AsyncMock(return_value=([], None)) + mock_qdrant.query_points = mocker.AsyncMock() + mock_qdrant.count = mocker.AsyncMock(return_value=CountResult(count=10)) + mocker.patch( + "vector_search.views.async_qdrant_client", + return_value=mock_qdrant, + ) + + params = { + "q": query_string, + "sortby": "-views", + "hybrid_search": hybrid_search, + } + + client.get( + reverse("vector_search:v0:vector_learning_resources_search"), data=params + ) + + if query_string: + call_kwargs = mock_qdrant.query_points.mock_calls[0].kwargs + assert isinstance(call_kwargs["query"], models.OrderByQuery) + assert call_kwargs["query"].order_by.key == "views" + assert call_kwargs["query"].order_by.direction == models.Direction.DESC + else: + call_kwargs = mock_qdrant.scroll.mock_calls[0].kwargs + assert "order_by" in call_kwargs + assert call_kwargs["order_by"].key == "views" + assert call_kwargs["order_by"].direction == models.Direction.DESC From f0bff074ab212c85a9216cd0819d1ad5a7454cff Mon Sep 17 00:00:00 2001 From: Carey P Gumaer Date: Thu, 23 Apr 2026 16:07:31 -0400 Subject: [PATCH 2/5] submit course / program as product_of_interest if the field is on the form (#3231) * submit title as product_of_interest if the field is on the form * fix issues with semicolons in titles as well as uniqueness with readable_id as the value for these options Co-authored-by: Copilot * handle issue with rebase conflict Co-authored-by: Copilot * hubspot maybe returns snake case, apparently Co-authored-by: Copilot * fix label deduping Co-authored-by: Copilot * don't return undefined from findProductOfInterestValue * rename learning-resource-field to option-label-field Co-authored-by: Copilot * use the existing resource prop Co-authored-by: Copilot * Make specifying an ETL source mandatory, and allow the user to specify more than one. Co-authored-by: Copilot * stray missed readableId --------- Co-authored-by: Copilot --- .../ProductPages/ProductPageTemplate.test.tsx | 9 +- .../ProductPages/ProductPageTemplate.tsx | 4 +- .../ProductPages/StayUpdatedModal.test.tsx | 172 +++++++++++++- .../ProductPages/StayUpdatedModal.tsx | 49 +++- ol_hubspot/api.py | 30 ++- ol_hubspot/api_test.py | 16 +- ...update_hubspot_contact_property_choices.py | 128 +++++++--- ...e_hubspot_contact_property_choices_test.py | 223 +++++++++++++++--- 8 files changed, 538 insertions(+), 93 deletions(-) diff --git a/frontends/main/src/app-pages/ProductPages/ProductPageTemplate.test.tsx b/frontends/main/src/app-pages/ProductPages/ProductPageTemplate.test.tsx index 5e04b557ae..9fd9351921 100644 --- a/frontends/main/src/app-pages/ProductPages/ProductPageTemplate.test.tsx +++ b/frontends/main/src/app-pages/ProductPages/ProductPageTemplate.test.tsx @@ -2,6 +2,7 @@ import React from "react" import { setMockResponse, urls, factories } from "api/test-utils" import { renderWithProviders, screen } from "@/test-utils" import ProductPageTemplate from "./ProductPageTemplate" +import { StayUpdatedModal } from "./StayUpdatedModal" import { useHubspotFormDetail } from "api/hooks/hubspot" import NiceModal from "@ebay/nice-modal-react" import { STAY_UPDATED_FORM_ID } from "./test-utils/stayUpdated" @@ -106,7 +107,9 @@ describe("ProductPageTemplate stay-updated trigger", () => { expect(button).toBeEnabled() button.click() - expect(mockedNiceModalShow).toHaveBeenCalled() + expect(mockedNiceModalShow).toHaveBeenCalledWith(StayUpdatedModal, { + productReadableId: DEFAULT_RESOURCE.readable_id, + }) }) it("disables the trigger button when form fetch errors", () => { @@ -140,7 +143,9 @@ describe("ProductPageTemplate stay-updated trigger", () => { expect(button).toBeEnabled() button.click() - expect(mockedNiceModalShow).toHaveBeenCalled() + expect(mockedNiceModalShow).toHaveBeenCalledWith(StayUpdatedModal, { + productReadableId: DEFAULT_RESOURCE.readable_id, + }) }) describe("PostHog tracking", () => { diff --git a/frontends/main/src/app-pages/ProductPages/ProductPageTemplate.tsx b/frontends/main/src/app-pages/ProductPages/ProductPageTemplate.tsx index 6774db14af..9c950555e3 100644 --- a/frontends/main/src/app-pages/ProductPages/ProductPageTemplate.tsx +++ b/frontends/main/src/app-pages/ProductPages/ProductPageTemplate.tsx @@ -309,7 +309,9 @@ const ProductPageTemplate: React.FC = ({ platform: PlatformEnum.Mitxonline, }) } - NiceModal.show(StayUpdatedModal) + NiceModal.show(StayUpdatedModal, { + productReadableId: resource.readable_id, + }) } return ( diff --git a/frontends/main/src/app-pages/ProductPages/StayUpdatedModal.test.tsx b/frontends/main/src/app-pages/ProductPages/StayUpdatedModal.test.tsx index 102cc2204d..d4282fbbca 100644 --- a/frontends/main/src/app-pages/ProductPages/StayUpdatedModal.test.tsx +++ b/frontends/main/src/app-pages/ProductPages/StayUpdatedModal.test.tsx @@ -1,7 +1,11 @@ import React from "react" import * as NiceModal from "@ebay/nice-modal-react" -import { HubspotForm, type HubspotFormProps } from "ol-components" -import { setMockResponse, urls, factories } from "api/test-utils" +import { + HubspotForm, + HubspotFormInput, + type HubspotFormProps, +} from "ol-components" +import { setMockResponse, urls, factories, makeRequest } from "api/test-utils" import { renderWithProviders, screen, user, act, waitFor } from "@/test-utils" import { StayUpdatedModal } from "./StayUpdatedModal" import { STAY_UPDATED_FORM_ID } from "./test-utils/stayUpdated" @@ -13,15 +17,23 @@ jest.mock("ol-components", () => ({ const mockedHubspotForm = jest.mocked(HubspotForm) const TEST_EMAIL = "user@test.edu" +const TEST_PRODUCT_TITLE = "Sample Program" +const TEST_PRODUCT_READABLE_ID = "sample-program" -const setupApis = () => { - setMockResponse.get( - urls.hubspot.details({ form_id: STAY_UPDATED_FORM_ID }), - factories.hubspot.form({ - id: STAY_UPDATED_FORM_ID, - name: "Stay Updated", - }), - ) +type HubspotFormOverride = + | Parameters[0] + | HubspotFormInput + +const setupApis = (formOverrides: HubspotFormOverride = {}) => { + const form = factories.hubspot.form({ + id: STAY_UPDATED_FORM_ID, + name: "Stay Updated", + }) + + setMockResponse.get(urls.hubspot.details({ form_id: STAY_UPDATED_FORM_ID }), { + ...form, + ...formOverrides, + }) setMockResponse.post(urls.hubspot.submit(STAY_UPDATED_FORM_ID), {}) } @@ -29,6 +41,7 @@ describe("StayUpdatedModal", () => { beforeEach(() => { process.env.NEXT_PUBLIC_STAY_UPDATED_HUBSPOT_FORM_ID = STAY_UPDATED_FORM_ID process.env.NEXT_PUBLIC_RECAPTCHA_SITE_KEY = "test-site-key" + makeRequest.mockClear() mockedHubspotForm.mockImplementation((props: HubspotFormProps) => (
@@ -157,6 +170,145 @@ describe("StayUpdatedModal", () => { }) }) + it("submits product_of_interest when the readable id matches a field option value", async () => { + setupApis({ + fieldGroups: [ + { + fields: [ + { + name: "product_of_interest", + label: "Product of Interest", + field_type: "multiple_checkboxes", + options: [ + { + label: TEST_PRODUCT_TITLE, + value: TEST_PRODUCT_READABLE_ID, + }, + ], + }, + ], + }, + ], + }) + renderWithProviders(null) + act(() => { + NiceModal.show(StayUpdatedModal, { + productReadableId: TEST_PRODUCT_READABLE_ID, + }) + }) + + await screen.findByRole("dialog", { name: "Stay Updated" }) + await user.click(screen.getByRole("button", { name: "Notify Me" })) + + expect(makeRequest).toHaveBeenCalledWith( + "post", + urls.hubspot.submit(STAY_UPDATED_FORM_ID), + expect.objectContaining({ + fields: expect.arrayContaining([ + { name: "email", value: TEST_EMAIL }, + { + name: "product_of_interest", + value: [TEST_PRODUCT_READABLE_ID], + }, + ]), + }), + ) + }) + + it("submits product_of_interest when the HubSpot response uses field_groups", async () => { + setupApis({ + field_groups: [ + { + fields: [ + { + name: "product_of_interest", + label: "Product of Interest", + field_type: "multiple_checkboxes", + options: [ + { + label: TEST_PRODUCT_TITLE, + value: TEST_PRODUCT_READABLE_ID, + }, + ], + }, + ], + }, + ], + }) + renderWithProviders(null) + act(() => { + NiceModal.show(StayUpdatedModal, { + productReadableId: TEST_PRODUCT_READABLE_ID, + }) + }) + + await screen.findByRole("dialog", { name: "Stay Updated" }) + await user.click(screen.getByRole("button", { name: "Notify Me" })) + + expect(makeRequest).toHaveBeenCalledWith( + "post", + urls.hubspot.submit(STAY_UPDATED_FORM_ID), + expect.objectContaining({ + fields: expect.arrayContaining([ + { name: "email", value: TEST_EMAIL }, + { + name: "product_of_interest", + value: [TEST_PRODUCT_READABLE_ID], + }, + ]), + }), + ) + }) + + it("omits product_of_interest when the readable id has no matching option value", async () => { + setupApis({ + fieldGroups: [ + { + fields: [ + { + name: "product_of_interest", + label: "Product of Interest", + field_type: "multiple_checkboxes", + options: [ + { + label: "Different Product", + value: "different_product", + }, + ], + }, + ], + }, + ], + }) + renderWithProviders(null) + act(() => { + NiceModal.show(StayUpdatedModal, { + productReadableId: TEST_PRODUCT_READABLE_ID, + }) + }) + + await screen.findByRole("dialog", { name: "Stay Updated" }) + await user.click(screen.getByRole("button", { name: "Notify Me" })) + + const postCall = makeRequest.mock.calls.find( + ([method, url]) => + method === "post" && url === urls.hubspot.submit(STAY_UPDATED_FORM_ID), + ) + expect(postCall).toBeDefined() + + const requestBody = postCall?.[2] as { + fields?: Array<{ name: string; value: unknown }> + } + expect(requestBody.fields).toEqual( + expect.arrayContaining([{ name: "email", value: TEST_EMAIL }]), + ) + expect(requestBody.fields).not.toEqual( + expect.arrayContaining([ + expect.objectContaining({ name: "product_of_interest" }), + ]), + ) + }) + it("shows error message when form submission fails", async () => { setMockResponse.get( urls.hubspot.details({ form_id: STAY_UPDATED_FORM_ID }), diff --git a/frontends/main/src/app-pages/ProductPages/StayUpdatedModal.tsx b/frontends/main/src/app-pages/ProductPages/StayUpdatedModal.tsx index c11f273e31..763df2728b 100644 --- a/frontends/main/src/app-pages/ProductPages/StayUpdatedModal.tsx +++ b/frontends/main/src/app-pages/ProductPages/StayUpdatedModal.tsx @@ -7,6 +7,7 @@ import { HubspotForm, Dialog, DialogActions, + type HubspotFormInput, type HubspotFormValue, } from "ol-components" import { Button, styled } from "@mitodl/smoot-design" @@ -43,6 +44,12 @@ const DialogSuccessCheck = styled(Image)({ marginBottom: "24px", }) +const PRODUCT_OF_INTEREST_FIELD_NAME = "product_of_interest" + +type StayUpdatedDialogProps = { + productReadableId?: string +} + const mapValuesToFields = ( values: Record, ): HubspotSubmitField[] => { @@ -54,7 +61,31 @@ const mapValuesToFields = ( .map(([name, value]) => ({ name, value })) } -const StayUpdatedDialogInner: React.FC = () => { +const findProductOfInterestValue = ( + hubspotForm: HubspotFormInput | undefined, + productReadableId: string | undefined, +): string | undefined => { + const normalizedProductReadableId = productReadableId?.trim() + if (!normalizedProductReadableId) { + return undefined + } + + const fieldGroups = hubspotForm?.fieldGroups ?? hubspotForm?.field_groups + + const productOfInterestField = fieldGroups + ?.flatMap((fieldGroup) => fieldGroup.fields ?? []) + .find((field) => field.name === PRODUCT_OF_INTEREST_FIELD_NAME) + + const matchingOption = productOfInterestField?.options?.find( + (option) => option.value?.trim() === normalizedProductReadableId, + ) + + return matchingOption?.value?.trim() +} + +const StayUpdatedDialogInner: React.FC = ({ + productReadableId, +}) => { const modalState = NiceModal.useModal() const modal = muiDialogV5(modalState) const stayUpdatedFormId = getStayUpdatedHubspotFormId() @@ -110,11 +141,25 @@ const StayUpdatedDialogInner: React.FC = () => { } onSubmit={(values, _event, recaptchaToken) => { - const fields = mapValuesToFields(values) + const fields = mapValuesToFields(values).filter( + (field) => field.name !== PRODUCT_OF_INTEREST_FIELD_NAME, + ) const emailField = fields.find((field) => field.name === "email") if (emailField && typeof emailField.value === "string") { setEmail(emailField.value) } + + const productOfInterestValue = findProductOfInterestValue( + hubspotForm, + productReadableId, + ) + if (productOfInterestValue) { + fields.push({ + name: PRODUCT_OF_INTEREST_FIELD_NAME, + value: [productOfInterestValue], + }) + } + hubspotFormSubmit.mutate({ formId: stayUpdatedFormId, fields, diff --git a/ol_hubspot/api.py b/ol_hubspot/api.py index cde2b0a41d..e4b62400f3 100644 --- a/ol_hubspot/api.py +++ b/ol_hubspot/api.py @@ -1,6 +1,7 @@ """HubSpot API helpers.""" from http import HTTPStatus +from typing import TypedDict import requests from django.conf import settings @@ -14,6 +15,13 @@ RECAPTCHA_VERIFY_URL = "https://www.google.com/recaptcha/api/siteverify" +class ContactPropertyOption(TypedDict): + """HubSpot contact property option payload.""" + + label: str + value: str + + def get_hubspot_client() -> HubSpot: """Create an authenticated HubSpot SDK client.""" return HubSpot(access_token=settings.MITOL_HUBSPOT_API_PRIVATE_TOKEN) @@ -54,16 +62,18 @@ def list_forms( ) -def _build_contact_property_options(option_values: list[str]) -> list[OptionInput]: - """Convert simple strings to HubSpot contact property option models.""" +def _build_contact_property_options( + options: list[ContactPropertyOption], +) -> list[OptionInput]: + """Convert label/value mappings to HubSpot contact property option models.""" return [ OptionInput( hidden=False, display_order=index, - label=value, - value=value, + label=option["label"], + value=option["value"], ) - for index, value in enumerate(option_values) + for index, option in enumerate(options) ] @@ -77,7 +87,7 @@ def create_contact_property( # noqa: PLR0913 *, property_name: str, label: str, - option_values: list[str], + options: list[ContactPropertyOption], group_name: str, field_type: str, description: str = "", @@ -93,15 +103,17 @@ def create_contact_property( # noqa: PLR0913 type="enumeration", field_type=field_type, form_field=form_field, - options=_build_contact_property_options(option_values), + options=_build_contact_property_options(options), ) return client.crm.properties.core_api.create("contacts", payload) -def update_contact_property_choices(*, property_name: str, option_values: list[str]): +def update_contact_property_choices( + *, property_name: str, options: list[ContactPropertyOption] +): """Update options on an existing HubSpot contact property.""" client = get_hubspot_client() - payload = PropertyUpdate(options=_build_contact_property_options(option_values)) + payload = PropertyUpdate(options=_build_contact_property_options(options)) return client.crm.properties.core_api.update("contacts", property_name, payload) diff --git a/ol_hubspot/api_test.py b/ol_hubspot/api_test.py index 271229c156..d489ee469b 100644 --- a/ol_hubspot/api_test.py +++ b/ol_hubspot/api_test.py @@ -118,7 +118,10 @@ def test_create_contact_property(mocker, settings, faker): create_contact_property( property_name=property_name, label=faker.sentence(nb_words=3), - option_values=["Course A", "Program B"], + options=[ + {"label": "Course A", "value": "course-a"}, + {"label": "Program B", "value": "program-b"}, + ], group_name="contactinformation", field_type="checkbox", description="Products a learner is interested in.", @@ -134,7 +137,8 @@ def test_create_contact_property(mocker, settings, faker): assert payload.type == "enumeration" assert payload.field_type == "checkbox" assert payload.form_field is True - assert [option.value for option in payload.options] == ["Course A", "Program B"] + assert [option.label for option in payload.options] == ["Course A", "Program B"] + assert [option.value for option in payload.options] == ["course-a", "program-b"] def test_update_contact_property_choices(mocker, settings, faker): @@ -146,7 +150,10 @@ def test_update_contact_property_choices(mocker, settings, faker): update_contact_property_choices( property_name=property_name, - option_values=["Course A", "Program B"], + options=[ + {"label": "Course A", "value": "course-a"}, + {"label": "Program B", "value": "program-b"}, + ], ) client.crm.properties.core_api.update.assert_called_once() @@ -154,7 +161,8 @@ def test_update_contact_property_choices(mocker, settings, faker): assert args[0] == "contacts" assert args[1] == property_name payload = args[2] - assert [option.value for option in payload.options] == ["Course A", "Program B"] + assert [option.label for option in payload.options] == ["Course A", "Program B"] + assert [option.value for option in payload.options] == ["course-a", "program-b"] def test_get_contact_property_propagates_sdk_error(mocker, settings, faker): diff --git a/ol_hubspot/management/commands/update_hubspot_contact_property_choices.py b/ol_hubspot/management/commands/update_hubspot_contact_property_choices.py index 0dcb7f7815..75bba694b2 100644 --- a/ol_hubspot/management/commands/update_hubspot_contact_property_choices.py +++ b/ol_hubspot/management/commands/update_hubspot_contact_property_choices.py @@ -4,13 +4,16 @@ import json from argparse import BooleanOptionalAction, RawTextHelpFormatter +from collections import Counter from django.core.management.base import BaseCommand, CommandError from hubspot.crm.properties.exceptions import ApiException as CrmPropertiesApiException from learning_resources.constants import LearningResourceType +from learning_resources.etl.constants import ETLSource from learning_resources.models import LearningResource from ol_hubspot.api import ( + ContactPropertyOption, create_contact_property, get_contact_property, update_contact_property_choices, @@ -55,12 +58,14 @@ def add_arguments(self, parser): "Examples:\n" " python manage.py update_hubspot_contact_property_choices " "learner_interest_choices \\\n" - " --learning-resource-field title \\\n" + " --option-label-field title \\\n" + " --etl-source mitxonline \\\n" " --resource-order-by title \\\n" " --property-field-type checkbox\n\n" " python manage.py update_hubspot_contact_property_choices " "learner_interest_choices \\\n" - " --learning-resource-field offered_by__name \\\n" + " --option-label-field offered_by__name \\\n" + " --etl-source xpro --etl-source mitxonline \\\n" " --resource-filter professional=true \\\n" " --resource-type course --resource-type program \\\n" " --resource-distinct\n\n" @@ -77,14 +82,35 @@ def add_arguments(self, parser): ) parser.add_argument( - "--learning-resource-field", - dest="learning_resource_field", + "--option-label-field", + dest="option_label_field", required=True, help=( - "LearningResource field/annotation to use for option values. " + "LearningResource field/annotation to use for option labels. " "Supports Django double-underscore traversal." ), ) + parser.add_argument( + "--option-value-field", + dest="option_value_field", + help=( + "LearningResource field/annotation to use for option values. " + "Defaults to --option-label-field." + ), + ) + + parser.add_argument( + "--etl-source", + dest="etl_sources", + action="append", + required=True, + choices=ETLSource.names(), + metavar="ETL_SOURCE", + help=( + "ETL source to include (required; repeat for multiple). " + f"Choices: {', '.join(ETLSource.names())}" + ), + ) parser.add_argument( "--resource-filter", @@ -177,14 +203,17 @@ def add_arguments(self, parser): def _resolve_choice_values_from_learning_resources( self, options: dict - ) -> list[str]: - """Build option values from LearningResource records.""" - value_field = options["learning_resource_field"] + ) -> list[tuple[object, object]]: + """Build option labels and values from LearningResource records.""" + label_field = options["option_label_field"] + value_field = options.get("option_value_field") or label_field queryset = LearningResource.objects.all() if not options["include_unpublished"]: queryset = queryset.filter(published=True) + queryset = queryset.filter(etl_source__in=options["etl_sources"]) + resource_types = options.get("resource_types") or [ LearningResourceType.course.name, LearningResourceType.program.name, @@ -204,35 +233,80 @@ def _resolve_choice_values_from_learning_resources( if order_by: queryset = queryset.order_by(*order_by) - values_qs = queryset.values_list(value_field, flat=True) + values_qs = queryset.values_list(label_field, value_field) if options.get("resource_distinct"): values_qs = values_qs.order_by().distinct() - return [str(value) for value in values_qs if value is not None] + return [ + (label, value) + for label, value in values_qs + if label is not None and value is not None + ] - def _resolve_choice_values(self, options: dict) -> list[str]: - """Resolve option values from LearningResource data.""" + def _resolve_choice_options(self, options: dict) -> list[ContactPropertyOption]: + """Resolve HubSpot options from LearningResource data.""" resource_values = self._resolve_choice_values_from_learning_resources(options) - values = [value.strip() for value in resource_values if value and value.strip()] + deduped_options: dict[str, str] = {} + for raw_label, raw_value in resource_values: + label = str(raw_label).strip() + value = str(raw_value).strip() + if not label or not value: + continue + deduped_options.setdefault(value, label) + + sorted_options = [ + {"label": label, "value": value} + for value, label in sorted( + deduped_options.items(), key=lambda item: item[1] + ) + ] + + return self._uniquify_option_labels(sorted_options) + + def _uniquify_option_labels( + self, options: list[ContactPropertyOption] + ) -> list[ContactPropertyOption]: + """Ensure labels are unique for HubSpot enumeration properties.""" + label_counts = Counter(option["label"] for option in options) + original_labels = {option["label"] for option in options} + used_labels: set[str] = set() + unique_options: list[ContactPropertyOption] = [] + + for option in options: + label = option["label"] + if label_counts[label] == 1: + unique_options.append(option) + used_labels.add(label) + continue + + candidate_label = f"{label} ({option['value']})" + suffix = 2 + while candidate_label in used_labels or candidate_label in original_labels: + candidate_label = f"{label} ({option['value']}) [{suffix}]" + suffix += 1 + + unique_options.append({**option, "label": candidate_label}) + used_labels.add(candidate_label) - return sorted(dict.fromkeys(values)) + return unique_options def handle(self, *args, **options): # noqa: ARG002 """Create or update one HubSpot contact property.""" property_name = options["property_name"] - option_values = self._resolve_choice_values(options) - if not option_values and not options["allow_empty"]: + property_options = self._resolve_choice_options(options) + if not property_options and not options["allow_empty"]: msg = ( "No choices were resolved from LearningResource data. " - "Adjust --learning-resource-field and filters, or pass " + "Adjust --option-label-field, --option-value-field, --etl-source, and " + "filters, or pass " "--allow-empty to intentionally clear options." ) raise CommandError(msg) self.stdout.write( - f"Resolved {len(option_values)} choices for property '{property_name}'." + f"Resolved {len(property_options)} choices for property '{property_name}'." ) if options["dry_run"]: @@ -245,11 +319,11 @@ def handle(self, *args, **options): # noqa: ARG002 [ { "displayOrder": index, - "label": value, - "value": value, + "label": option["label"], + "value": option["value"], "hidden": False, } - for index, value in enumerate(option_values) + for index, option in enumerate(property_options) ], indent=2, ) @@ -264,7 +338,7 @@ def handle(self, *args, **options): # noqa: ARG002 else: msg = ( "Unable to read HubSpot contact property " - f"'{property_name}': {exc.reason}" + f"'{property_name}': {exc.reason} - {exc.body}" ) raise CommandError(msg) from exc @@ -286,7 +360,7 @@ def handle(self, *args, **options): # noqa: ARG002 create_contact_property( property_name=property_name, label=property_label, - option_values=option_values, + options=property_options, group_name=options["property_group_name"], field_type=options["property_field_type"], description=options["property_description"], @@ -294,21 +368,21 @@ def handle(self, *args, **options): # noqa: ARG002 ) result_message = ( f"Created contact property '{property_name}' with " - f"{len(option_values)} choices." + f"{len(property_options)} choices." ) else: update_contact_property_choices( property_name=property_name, - option_values=option_values, + options=property_options, ) result_message = ( f"Updated contact property '{property_name}' with " - f"{len(option_values)} choices." + f"{len(property_options)} choices." ) except CrmPropertiesApiException as exc: msg = ( "Unable to sync HubSpot contact property " - f"'{property_name}': {exc.reason}" + f"'{property_name}': {exc.reason} - {exc.body}" ) raise CommandError(msg) from exc diff --git a/ol_hubspot/management/commands/update_hubspot_contact_property_choices_test.py b/ol_hubspot/management/commands/update_hubspot_contact_property_choices_test.py index f01c1a3574..6b8c1716c0 100644 --- a/ol_hubspot/management/commands/update_hubspot_contact_property_choices_test.py +++ b/ol_hubspot/management/commands/update_hubspot_contact_property_choices_test.py @@ -6,10 +6,7 @@ from django.core.management import CommandError, call_command from hubspot.crm.properties.exceptions import ApiException as CrmPropertiesApiException -from learning_resources.factories import ( - LearningResourceFactory, - LearningResourceOfferorFactory, -) +from learning_resources.factories import LearningResourceFactory @pytest.mark.django_db @@ -19,12 +16,14 @@ def test_command_creates_contact_property_when_missing(mocker, faker): LearningResourceFactory( is_course=True, title="Algorithms", - etl_source="hubspot_test", + readable_id="algorithms", + etl_source="mitxonline", ) LearningResourceFactory( is_program=True, title="Data Science Program", - etl_source="hubspot_test", + readable_id="data-science-program", + etl_source="mitxonline", ) mocker.patch( @@ -42,10 +41,12 @@ def test_command_creates_contact_property_when_missing(mocker, faker): call_command( "update_hubspot_contact_property_choices", property_name, - "--learning-resource-field", + "--option-label-field", "title", - "--resource-filter", - "etl_source=hubspot_test", + "--option-value-field", + "readable_id", + "--etl-source", + "mitxonline", "--resource-order-by", "title", stdout=stdout, @@ -56,32 +57,27 @@ def test_command_creates_contact_property_when_missing(mocker, faker): kwargs = create_mock.call_args.kwargs assert kwargs["property_name"] == property_name assert kwargs["label"] == property_name.replace("_", " ").title() - assert kwargs["option_values"] == ["Algorithms", "Data Science Program"] + assert kwargs["options"] == [ + {"label": "Algorithms", "value": "algorithms"}, + {"label": "Data Science Program", "value": "data-science-program"}, + ] @pytest.mark.django_db def test_command_updates_existing_contact_property(mocker, faker): """Command updates options when the target contact property already exists.""" property_name = faker.slug().replace("-", "_") - xpro = LearningResourceOfferorFactory(is_xpro=True) - ocw = LearningResourceOfferorFactory(is_ocw=True) LearningResourceFactory( is_course=True, title="AI Foundations", - offered_by=xpro, - etl_source="hubspot_test", + readable_id="ai-foundations", + etl_source="mitxonline", ) LearningResourceFactory( is_program=True, title="AI Program", - offered_by=xpro, - etl_source="hubspot_test", - ) - LearningResourceFactory( - is_course=True, - title="AI for Everyone", - offered_by=ocw, - etl_source="hubspot_test", + readable_id="ai-program", + etl_source="mitxonline", ) existing_property = mocker.Mock() @@ -100,23 +96,174 @@ def test_command_updates_existing_contact_property(mocker, faker): call_command( "update_hubspot_contact_property_choices", property_name, - "--learning-resource-field", - "offered_by__name", + "--option-label-field", + "title", + "--option-value-field", + "readable_id", + "--etl-source", + "mitxonline", "--resource-filter", "title__icontains=AI", - "--resource-filter", - "professional=true", - "--resource-filter", - "etl_source=hubspot_test", "--resource-order-by", - "offered_by__name", + "title", "--resource-distinct", ) create_mock.assert_not_called() update_mock.assert_called_once_with( property_name=property_name, - option_values=[xpro.name], + options=[ + {"label": "AI Foundations", "value": "ai-foundations"}, + {"label": "AI Program", "value": "ai-program"}, + ], + ) + + +@pytest.mark.django_db +def test_command_disambiguates_duplicate_labels_with_value_suffix(mocker, faker): + """Command makes duplicate labels unique for HubSpot while preserving values.""" + property_name = faker.slug().replace("-", "_") + LearningResourceFactory( + is_course=True, + title="Generative AI Playbook", + readable_id="gai-playbook-a", + etl_source="mitxonline", + ) + LearningResourceFactory( + is_program=True, + title="Generative AI Playbook", + readable_id="gai-playbook-b", + etl_source="mitxonline", + ) + + mocker.patch( + "ol_hubspot.management.commands.update_hubspot_contact_property_choices.get_contact_property", + side_effect=CrmPropertiesApiException(status=404, reason="Not Found"), + ) + create_mock = mocker.patch( + "ol_hubspot.management.commands.update_hubspot_contact_property_choices.create_contact_property" + ) + + call_command( + "update_hubspot_contact_property_choices", + property_name, + "--option-label-field", + "title", + "--option-value-field", + "readable_id", + "--etl-source", + "mitxonline", + "--resource-order-by", + "title", + ) + + create_mock.assert_called_once_with( + property_name=property_name, + label=property_name.replace("_", " ").title(), + options=[ + { + "label": "Generative AI Playbook (gai-playbook-a)", + "value": "gai-playbook-a", + }, + { + "label": "Generative AI Playbook (gai-playbook-b)", + "value": "gai-playbook-b", + }, + ], + group_name="contactinformation", + field_type="checkbox", + description="", + form_field=True, + ) + + +@pytest.mark.django_db +def test_command_avoids_collisions_with_existing_original_labels(mocker, faker): + """Disambiguated labels should not collide with any pre-existing label.""" + property_name = faker.slug().replace("-", "_") + LearningResourceFactory( + is_course=True, + title="Test", + readable_id="a", + etl_source="mitxonline", + ) + LearningResourceFactory( + is_program=True, + title="Test", + readable_id="b", + etl_source="mitxonline", + ) + LearningResourceFactory( + is_program=True, + title="Test (b)", + readable_id="c", + etl_source="mitxonline", + ) + + mocker.patch( + "ol_hubspot.management.commands.update_hubspot_contact_property_choices.get_contact_property", + side_effect=CrmPropertiesApiException(status=404, reason="Not Found"), + ) + create_mock = mocker.patch( + "ol_hubspot.management.commands.update_hubspot_contact_property_choices.create_contact_property" + ) + + call_command( + "update_hubspot_contact_property_choices", + property_name, + "--option-label-field", + "title", + "--option-value-field", + "readable_id", + "--etl-source", + "mitxonline", + "--resource-order-by", + "title", + ) + + create_mock.assert_called_once() + labels = [option["label"] for option in create_mock.call_args.kwargs["options"]] + assert len(labels) == len(set(labels)) + assert "Test (a)" in labels + assert "Test (b)" in labels + assert "Test (b) [2]" in labels + + +@pytest.mark.django_db +def test_command_uses_label_field_as_value_field_by_default(mocker, faker): + """Command preserves the current label=value behavior unless overridden.""" + property_name = faker.slug().replace("-", "_") + LearningResourceFactory( + is_course=True, + title="Algorithms", + etl_source="mitxonline", + ) + + mocker.patch( + "ol_hubspot.management.commands.update_hubspot_contact_property_choices.get_contact_property", + side_effect=CrmPropertiesApiException(status=404, reason="Not Found"), + ) + create_mock = mocker.patch( + "ol_hubspot.management.commands.update_hubspot_contact_property_choices.create_contact_property" + ) + + call_command( + "update_hubspot_contact_property_choices", + property_name, + "--option-label-field", + "title", + "--etl-source", + "mitxonline", + ) + + create_mock.assert_called_once_with( + property_name=property_name, + label=property_name.replace("_", " ").title(), + options=[{"label": "Algorithms", "value": "Algorithms"}], + group_name="contactinformation", + field_type="checkbox", + description="", + form_field=True, ) @@ -124,7 +271,7 @@ def test_command_updates_existing_contact_property(mocker, faker): def test_command_rejects_non_enumeration_existing_property(mocker, faker): """Command errors if an existing property is not an enumeration.""" property_name = faker.slug().replace("-", "_") - LearningResourceFactory(is_course=True, title="Algorithms") + LearningResourceFactory(is_course=True, title="Algorithms", etl_source="mitxonline") existing_property = mocker.Mock() existing_property.type = "string" @@ -137,8 +284,10 @@ def test_command_rejects_non_enumeration_existing_property(mocker, faker): call_command( "update_hubspot_contact_property_choices", property_name, - "--learning-resource-field", + "--option-label-field", "title", + "--etl-source", + "mitxonline", ) assert "unsupported type" in str(exc_info.value) @@ -148,9 +297,7 @@ def test_command_rejects_non_enumeration_existing_property(mocker, faker): def test_command_dry_run_skips_hubspot_lookup(mocker, faker): """Dry-run should not read HubSpot, even if lookup would fail.""" property_name = faker.slug().replace("-", "_") - LearningResourceFactory( - is_course=True, title="Algorithms", etl_source="hubspot_test" - ) + LearningResourceFactory(is_course=True, title="Algorithms", etl_source="mitxonline") get_property_mock = mocker.patch( "ol_hubspot.management.commands.update_hubspot_contact_property_choices.get_contact_property", @@ -168,10 +315,10 @@ def test_command_dry_run_skips_hubspot_lookup(mocker, faker): call_command( "update_hubspot_contact_property_choices", property_name, - "--learning-resource-field", + "--option-label-field", "title", - "--resource-filter", - "etl_source=hubspot_test", + "--etl-source", + "mitxonline", "--dry-run", ) From 0da5acff91c466f96cacb731baddac3c56161a73 Mon Sep 17 00:00:00 2001 From: Chris Chudzicki Date: Thu, 23 Apr 2026 17:34:48 -0400 Subject: [PATCH 3/5] Fix program dashboard "completed x of y" counts (#3217) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix: cap elective completions at min_number_of value in program progress counter The 'You have completed X of Y courses' header was counting all completed courses across all sections, including electives. For min_number_of sections, completions are now capped at operator_value — matching how totalCount already worked — so completing extra electives no longer inflates the required-course completion count. Also caps the per-section header ('Completed X of Y') for consistency, and extracts the counting logic into getProgramCounts / isRequirementSectionItemCompleted in helpers.ts with full unit test coverage. Fixes #10784 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * test: remove unnecessary title overrides from course factory calls Co-Authored-By: Claude Sonnet 4.6 * refactor: replace getProgramCounts with node-based getRequirementsProgress Counts now operate directly on req_tree operator nodes + enrollment lookup dicts, eliminating the UI-shaped RequirementSection intermediate from the counting path. The same function handles overall program progress (pass all top-level operators) and per-section progress (pass one operator). Nested operators are out of scope by design: only direct course/program children of an operator count as its requirements. `program-enrollment` items (sub-programs where display_mode !== Course) now count like any other program leaf — previously excluded — which is the more natural behavior. This data configuration is not in use. Co-Authored-By: Claude Opus 4.7 (1M context) * docs: clarify getRequirementsProgress contract around flat req_tree Co-Authored-By: Claude Opus 4.7 (1M context) * fix: skip + warn on unsupported operators instead of falling back to all_of Malformed min_number_of values (non-numeric operator_value) and unknown operators now contribute nothing to the progress count and log a warning. Previous behavior silently treated them like all_of, which over-reported. Co-Authored-By: Claude Opus 4.7 (1M context) * log a warning on nested children --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: Claude Sonnet 4.6 --- .../EnrollmentDisplay.test.tsx | 227 ++++++++++++++++++ .../CoursewareDisplay/EnrollmentDisplay.tsx | 135 +++++------ .../CoursewareDisplay/helpers.test.tsx | 168 ++++++++++++- .../CoursewareDisplay/helpers.ts | 93 +++++++ 4 files changed, 541 insertions(+), 82 deletions(-) diff --git a/frontends/main/src/app-pages/DashboardPage/CoursewareDisplay/EnrollmentDisplay.test.tsx b/frontends/main/src/app-pages/DashboardPage/CoursewareDisplay/EnrollmentDisplay.test.tsx index 3dff6a572a..1ff5215871 100644 --- a/frontends/main/src/app-pages/DashboardPage/CoursewareDisplay/EnrollmentDisplay.test.tsx +++ b/frontends/main/src/app-pages/DashboardPage/CoursewareDisplay/EnrollmentDisplay.test.tsx @@ -1739,6 +1739,233 @@ describe("EnrollmentDisplay", () => { }, ) + test("Overall completion count caps elective completions at min_number_of value", async () => { + /** + * Program has 2 required courses (all_of) and 3 electives (min_number_of=1). + * User completes 1 required course + 2 electives. + * Bug: counts all 3 completions → "3 of 3 courses" (wrong). + * Fix: caps elective contribution at 1 → "2 of 3 courses" (correct). + */ + const mitxOnlineUser = mitxonline.factories.user.user() + setMockResponse.get(mitxonline.urls.userMe.get(), mitxOnlineUser) + + const reqTree = + new mitxonline.factories.requirements.RequirementTreeBuilder() + const required = reqTree.addOperator({ + operator: "all_of", + title: "Required Courses", + }) + required.addCourse({ course: 1 }) + required.addCourse({ course: 2 }) + + const electives = reqTree.addOperator({ + operator: "min_number_of", + operator_value: "1", + title: "Electives", + }) + electives.addCourse({ course: 3 }) + electives.addCourse({ course: 4 }) + electives.addCourse({ course: 5 }) + + const program = mitxonline.factories.programs.program({ + id: 5555, + courses: [1, 2, 3, 4, 5], + req_tree: reqTree.serialize(), + }) + + const run1 = mitxonline.factories.courses.courseRun({ id: 101 }) + const run3 = mitxonline.factories.courses.courseRun({ id: 103 }) + const run4 = mitxonline.factories.courses.courseRun({ id: 104 }) + + const courses = { + count: 5, + next: null, + previous: null, + results: [ + mitxonline.factories.courses.course({ + id: 1, + courseruns: [run1], + }), + mitxonline.factories.courses.course({ + id: 2, + courseruns: [mitxonline.factories.courses.courseRun({ id: 102 })], + }), + mitxonline.factories.courses.course({ + id: 3, + courseruns: [run3], + }), + mitxonline.factories.courses.course({ + id: 4, + courseruns: [run4], + }), + mitxonline.factories.courses.course({ + id: 5, + courseruns: [mitxonline.factories.courses.courseRun({ id: 105 })], + }), + ], + } + + // Course 1 (required) completed, courses 3 and 4 (electives) completed + const completedGrade = mitxonline.factories.enrollment.grade({ + passed: true, + }) + const enrollments = [ + mitxonline.factories.enrollment.courseEnrollment({ + run: { ...run1, course: courses.results[0] }, + grades: [completedGrade], + }), + mitxonline.factories.enrollment.courseEnrollment({ + run: { ...run3, course: courses.results[2] }, + grades: [completedGrade], + }), + mitxonline.factories.enrollment.courseEnrollment({ + run: { ...run4, course: courses.results[3] }, + grades: [completedGrade], + }), + ] + + mockedUseFeatureFlagEnabled.mockReturnValue(true) + setMockResponse.get( + mitxonline.urls.enrollment.enrollmentsListV3(), + enrollments, + ) + setMockResponse.get( + mitxonline.urls.programEnrollments.enrollmentsListV3(), + [ + mitxonline.factories.enrollment.programEnrollmentV3({ + program: { + id: program.id, + title: program.title, + live: program.live, + program_type: program.program_type, + readable_id: program.readable_id, + }, + }), + ], + ) + setMockResponse.get(mitxonline.urls.programs.programDetail(5555), program) + setMockResponse.get( + mitxonline.urls.courses.coursesList({ + id: program.courses, + page_size: program.courses.length, + }), + courses, + ) + + renderWithProviders() + + // 1 required completed + min(2 electives completed, 1 required) = 2 total completed + // total = 2 required + 1 elective min = 3 + await screen.findByText(/2 of 3 courses/) + }) + + test("Section header caps displayed count at operator_value for min_number_of sections", async () => { + /** + * Electives section with min_number_of=1 and 3 completed courses. + * The section header should show "Completed 1 of 1", not "Completed 3 of 1". + */ + const mitxOnlineUser = mitxonline.factories.user.user() + setMockResponse.get(mitxonline.urls.userMe.get(), mitxOnlineUser) + + const reqTree = + new mitxonline.factories.requirements.RequirementTreeBuilder() + const electives = reqTree.addOperator({ + operator: "min_number_of", + operator_value: "1", + title: "Electives", + }) + electives.addCourse({ course: 1 }) + electives.addCourse({ course: 2 }) + electives.addCourse({ course: 3 }) + + const program = mitxonline.factories.programs.program({ + id: 6666, + courses: [1, 2, 3], + req_tree: reqTree.serialize(), + }) + + const run1 = mitxonline.factories.courses.courseRun({ id: 201 }) + const run2 = mitxonline.factories.courses.courseRun({ id: 202 }) + const run3 = mitxonline.factories.courses.courseRun({ id: 203 }) + + const courses = { + count: 3, + next: null, + previous: null, + results: [ + mitxonline.factories.courses.course({ + id: 1, + courseruns: [run1], + }), + mitxonline.factories.courses.course({ + id: 2, + courseruns: [run2], + }), + mitxonline.factories.courses.course({ + id: 3, + courseruns: [run3], + }), + ], + } + + // All 3 electives completed + const completedGrade = mitxonline.factories.enrollment.grade({ + passed: true, + }) + const enrollments = [ + mitxonline.factories.enrollment.courseEnrollment({ + run: { ...run1, course: courses.results[0] }, + grades: [completedGrade], + }), + mitxonline.factories.enrollment.courseEnrollment({ + run: { ...run2, course: courses.results[1] }, + grades: [completedGrade], + }), + mitxonline.factories.enrollment.courseEnrollment({ + run: { ...run3, course: courses.results[2] }, + grades: [completedGrade], + }), + ] + + mockedUseFeatureFlagEnabled.mockReturnValue(true) + setMockResponse.get( + mitxonline.urls.enrollment.enrollmentsListV3(), + enrollments, + ) + setMockResponse.get( + mitxonline.urls.programEnrollments.enrollmentsListV3(), + [ + mitxonline.factories.enrollment.programEnrollmentV3({ + program: { + id: program.id, + title: program.title, + live: program.live, + program_type: program.program_type, + readable_id: program.readable_id, + }, + }), + ], + ) + setMockResponse.get(mitxonline.urls.programs.programDetail(6666), program) + setMockResponse.get( + mitxonline.urls.courses.coursesList({ + id: program.courses, + page_size: program.courses.length, + }), + courses, + ) + + renderWithProviders() + + await screen.findByText("Electives") + + // Section header should show "Completed 1 of 1", capped at operator_value + const sectionCount = screen.getByTestId("section-completion-count") + expect(sectionCount).toHaveTextContent("Completed 1 of 1") + // Overall header should also show 1 of 1 (only electives section) + expect(screen.getByText(/1 of 1 courses/)).toBeInTheDocument() + }) + test("Returns 404 page when user is not enrolled in the program", async () => { const mitxOnlineUser = mitxonline.factories.user.user() setMockResponse.get(mitxonline.urls.userMe.get(), mitxOnlineUser) diff --git a/frontends/main/src/app-pages/DashboardPage/CoursewareDisplay/EnrollmentDisplay.tsx b/frontends/main/src/app-pages/DashboardPage/CoursewareDisplay/EnrollmentDisplay.tsx index c357d3f719..e89d7a66ed 100644 --- a/frontends/main/src/app-pages/DashboardPage/CoursewareDisplay/EnrollmentDisplay.tsx +++ b/frontends/main/src/app-pages/DashboardPage/CoursewareDisplay/EnrollmentDisplay.tsx @@ -18,7 +18,7 @@ import { keepPreviousData, useQuery } from "@tanstack/react-query" import { EnrollmentStatus, getEnrollmentStatus, - getProgramEnrollmentStatus, + getRequirementsProgress, getKey, ResourceType, selectBestEnrollment, @@ -308,6 +308,35 @@ interface ResourceItem { type: "course" | "program" } +type CourseRequirementItem = { + resourceType: "course" + course: CourseWithCourseRunsSerializerV2 +} + +type ProgramAsCourseRequirementItem = { + resourceType: "program-as-course" + courseProgramId: number + courseProgram: V2ProgramDetail + courseProgramEnrollment: V3UserProgramEnrollment | undefined +} + +type ProgramEnrollmentRequirementItem = { + resourceType: "program-enrollment" + enrollment: V3UserProgramEnrollment +} + +type RequirementSectionItem = + | CourseRequirementItem + | ProgramAsCourseRequirementItem + | ProgramEnrollmentRequirementItem + +type RequirementSection = { + key: string | number | null | undefined + title: string + items: RequirementSectionItem[] + node: V2ProgramRequirement +} + const extractResourcesFromNode = ( node: V2ProgramRequirement, ): ResourceItem[] => { @@ -436,7 +465,15 @@ const ProgramEnrollmentDisplay: React.FC = ({ {} as Record, ) - const requirementSections = + const programEnrollmentsById = (programEnrollments ?? []).reduce( + (acc, enrollment) => { + acc[enrollment.program.id] = enrollment + return acc + }, + {} as Record, + ) + + const requirementSections: RequirementSection[] = program?.req_tree .filter((node) => node.data.node_type === "operator") .map((node) => { @@ -444,12 +481,6 @@ const ProgramEnrollmentDisplay: React.FC = ({ (programCourses?.results ?? []).map((c) => [c.id, c]), ) const programsById = new Map(requiredProgramList.map((p) => [p.id, p])) - const programEnrollmentsById = new Map( - (programEnrollments ?? []).map((enrollment) => [ - enrollment.program.id, - enrollment, - ]), - ) const sectionItems = extractResourcesFromNode(node) .map((resource) => { @@ -473,13 +504,12 @@ const ProgramEnrollmentDisplay: React.FC = ({ resourceType: "program-as-course" as const, courseProgramId: requiredProgram.id, courseProgram: requiredProgram, - courseProgramEnrollment: programEnrollmentsById.get( - requiredProgram.id, - ), + courseProgramEnrollment: + programEnrollmentsById[requiredProgram.id], } } - const enrollment = programEnrollmentsById.get(requiredProgram.id) + const enrollment = programEnrollmentsById[requiredProgram.id] if (!enrollment) return null return { @@ -513,44 +543,12 @@ const ProgramEnrollmentDisplay: React.FC = ({ ) }, [requiredProgramList, requiredProgramCourses?.results]) - const completedCount = requirementSections - .flatMap((section) => section.items) - .filter((item) => { - if (item.resourceType === "course") { - const bestEnrollment = selectBestEnrollment( - item.course, - enrollmentsByCourseId[item.course.id] || [], - ) - return ( - getEnrollmentStatus(bestEnrollment) === EnrollmentStatus.Completed - ) - } - if (item.resourceType === "program-as-course") { - return ( - getProgramEnrollmentStatus(item.courseProgramEnrollment, 0, 0) === - EnrollmentStatus.Completed - ) - } - return false - }).length - - const totalCount = requirementSections.reduce((sum, section) => { - if ( - section.node.data.operator === "min_number_of" && - section.node.data.operator_value - ) { - return sum + parseInt(section.node.data.operator_value, 10) - } - return ( - sum + - section.items.filter((item) => { - return ( - item.resourceType === "course" || - item.resourceType === "program-as-course" - ) - }).length + const { completed: completedCount, total: totalCount } = + getRequirementsProgress( + requirementSections.map((s) => s.node), + enrollmentsByCourseId, + programEnrollmentsById, ) - }, 0) if (isLoading) { return ( @@ -590,35 +588,12 @@ const ProgramEnrollmentDisplay: React.FC = ({ {requirementSections.map((section, index) => { - const sectionCompletedCount = section.items.filter((item) => { - if (item.resourceType === "course") { - const bestEnrollment = selectBestEnrollment( - item.course, - enrollmentsByCourseId[item.course.id] || [], - ) - return ( - getEnrollmentStatus(bestEnrollment) === EnrollmentStatus.Completed - ) - } - if (item.resourceType === "program-as-course") { - return ( - getProgramEnrollmentStatus(item.courseProgramEnrollment, 0, 0) === - EnrollmentStatus.Completed - ) - } - return false - }).length - - const sectionRequiredCount = - section.node.data.operator === "min_number_of" && - section.node.data.operator_value - ? parseInt(section.node.data.operator_value, 10) - : section.items.filter((item) => { - return ( - item.resourceType === "course" || - item.resourceType === "program-as-course" - ) - }).length + const { completed: sectionCompleted, total: sectionTotal } = + getRequirementsProgress( + [section.node], + enrollmentsByCourseId, + programEnrollmentsById, + ) return ( @@ -635,13 +610,13 @@ const ProgramEnrollmentDisplay: React.FC = ({ > {section.title} - {sectionRequiredCount > 0 ? ( + {sectionTotal > 0 ? ( - Completed {sectionCompletedCount} of {sectionRequiredCount} + Completed {sectionCompleted} of {sectionTotal} ) : null} diff --git a/frontends/main/src/app-pages/DashboardPage/CoursewareDisplay/helpers.test.tsx b/frontends/main/src/app-pages/DashboardPage/CoursewareDisplay/helpers.test.tsx index 4d69478f01..369283c9e4 100644 --- a/frontends/main/src/app-pages/DashboardPage/CoursewareDisplay/helpers.test.tsx +++ b/frontends/main/src/app-pages/DashboardPage/CoursewareDisplay/helpers.test.tsx @@ -1,14 +1,21 @@ import { factories } from "api/mitxonline-test-utils" +import { + CourseRunEnrollmentV3, + V2ProgramRequirement, + V3UserProgramEnrollment, +} from "@mitodl/mitxonline-api-axios/v2" import { EnrollmentStatus, filterEnrollmentsByOrganization, getBestRun, - selectBestEnrollment, - getEnrollmentStatus, + getRequirementsProgress, getProgramEnrollmentStatus, getKey, + selectBestEnrollment, + getEnrollmentStatus, ResourceType, } from "./helpers" +import { allowConsoleErrors } from "ol-test-utilities" describe("helpers", () => { describe("getKey", () => { @@ -450,4 +457,161 @@ describe("helpers", () => { ) }) }) + + describe("getRequirementsProgress", () => { + const courseLeaf = (id: number): V2ProgramRequirement => ({ + data: { node_type: "course", course: id }, // eslint-disable-line camelcase + }) + + const programLeaf = (id: number): V2ProgramRequirement => ({ + data: { node_type: "program", required_program: id }, // eslint-disable-line camelcase + }) + + const operator = ( + op: string, + children: V2ProgramRequirement[], + operatorValue: string | null = null, + ): V2ProgramRequirement => ({ + data: { + node_type: "operator", + operator: op, + operator_value: operatorValue, // eslint-disable-line camelcase + }, + children, + }) + + const courseEnrollment = ( + courseId: number, + passed: boolean, + ): CourseRunEnrollmentV3 => { + const run = factories.courses.courseRun({ id: courseId }) + const course = factories.courses.course({ + id: courseId, + courseruns: [run], + }) + return factories.enrollment.courseEnrollment({ + run: { ...run, course }, + grades: [factories.enrollment.grade({ passed })], + }) + } + + test("all_of counts completed courses against total children", () => { + const nodes = [operator("all_of", [courseLeaf(1), courseLeaf(2)])] + const courseEnrollments = { + 1: [courseEnrollment(1, true)], + 2: [courseEnrollment(2, false)], + } + + expect(getRequirementsProgress(nodes, courseEnrollments, {})).toEqual({ + completed: 1, + total: 2, + }) + }) + + test("min_number_of caps completed and total at operator_value", () => { + const nodes = [ + operator( + "min_number_of", + [courseLeaf(1), courseLeaf(2), courseLeaf(3)], + "1", + ), + ] + const courseEnrollments = { + 1: [courseEnrollment(1, true)], + 2: [courseEnrollment(2, true)], + 3: [courseEnrollment(3, true)], + } + + expect(getRequirementsProgress(nodes, courseEnrollments, {})).toEqual({ + completed: 1, + total: 1, + }) + }) + + test("sums progress across multiple top-level operators", () => { + const nodes = [ + operator("all_of", [courseLeaf(1), courseLeaf(2)]), + operator( + "min_number_of", + [courseLeaf(3), courseLeaf(4), courseLeaf(5)], + "1", + ), + ] + const courseEnrollments = { + 1: [courseEnrollment(1, true)], + 3: [courseEnrollment(3, true)], + 4: [courseEnrollment(4, true)], + } + + expect(getRequirementsProgress(nodes, courseEnrollments, {})).toEqual({ + completed: 2, + total: 3, + }) + }) + + test("program leaf is completed when enrollment has certificate", () => { + const enrollments: Record = { + 10: factories.enrollment.programEnrollmentV3({ + certificate: { uuid: "cert-uuid" }, + }), + } + const nodes = [operator("all_of", [programLeaf(10), programLeaf(11)])] + + expect(getRequirementsProgress(nodes, {}, enrollments)).toEqual({ + completed: 1, + total: 2, + }) + }) + + test("ignores non-operator and non-leaf children (nested operators)", () => { + // Nested operators are not counted — only direct course/program children + const nested = operator("all_of", [courseLeaf(99)]) + const nodes = [operator("all_of", [courseLeaf(1), nested])] + + const { consoleWarn } = allowConsoleErrors() + expect( + getRequirementsProgress(nodes, { 1: [courseEnrollment(1, true)] }, {}), + ).toEqual({ completed: 1, total: 1 }) + expect(consoleWarn).toHaveBeenCalled() + }) + + test("min_number_of with operator_value=0 contributes nothing", () => { + const nodes = [operator("min_number_of", [courseLeaf(1)], "0")] + + expect( + getRequirementsProgress(nodes, { 1: [courseEnrollment(1, true)] }, {}), + ).toEqual({ completed: 0, total: 0 }) + }) + + test("min_number_of with malformed operator_value contributes nothing and warns", () => { + const warn = jest.spyOn(console, "warn").mockImplementation(() => {}) + const nodes = [operator("min_number_of", [courseLeaf(1)], "not-a-number")] + + expect( + getRequirementsProgress(nodes, { 1: [courseEnrollment(1, true)] }, {}), + ).toEqual({ completed: 0, total: 0 }) + expect(warn).toHaveBeenCalled() + warn.mockRestore() + }) + + test("unknown operator contributes nothing and warns", () => { + const warn = jest.spyOn(console, "warn").mockImplementation(() => {}) + const nodes = [ + operator("at_least_one_of", [courseLeaf(1), courseLeaf(2)]), + ] + + expect( + getRequirementsProgress(nodes, { 1: [courseEnrollment(1, true)] }, {}), + ).toEqual({ completed: 0, total: 0 }) + expect(warn).toHaveBeenCalled() + warn.mockRestore() + }) + + test("empty nodes returns zeroes", () => { + expect(getRequirementsProgress([], {}, {})).toEqual({ + completed: 0, + total: 0, + }) + }) + }) }) diff --git a/frontends/main/src/app-pages/DashboardPage/CoursewareDisplay/helpers.ts b/frontends/main/src/app-pages/DashboardPage/CoursewareDisplay/helpers.ts index 06ed0f8ce8..1e26fe98af 100644 --- a/frontends/main/src/app-pages/DashboardPage/CoursewareDisplay/helpers.ts +++ b/frontends/main/src/app-pages/DashboardPage/CoursewareDisplay/helpers.ts @@ -1,6 +1,7 @@ import { CourseRunEnrollmentV3, CourseWithCourseRunsSerializerV2, + V2ProgramRequirement, V3UserProgramEnrollment, } from "@mitodl/mitxonline-api-axios/v2" import { getBestRun } from "@/common/mitxonline" @@ -110,3 +111,95 @@ export { getEnrollmentStatus, getProgramEnrollmentStatus, } + +const isLeafRequirementNodeCompleted = ( + node: V2ProgramRequirement, + courseEnrollments: Record, + programEnrollments: Record, +): boolean => { + if ( + node.data.node_type === "course" && + typeof node.data.course === "number" + ) { + const enrollments = courseEnrollments[node.data.course] ?? [] + return enrollments.some((e) => e.grades.some((g) => g.passed)) + } + if (node.data.node_type === "program" && node.data.required_program) { + return !!programEnrollments[node.data.required_program]?.certificate + } + return false +} + +/** + * Computes `{ completed, total }` across the given operator nodes. + * + * Assumes a flat req_tree: each operator's direct children are leaves + * (`node_type: "course"` or `"program"`). Nesting operators inside + * operators is not supported — there is no single well-defined reduction + * for nested progress (e.g., with `min_number_of=1` parent over two + * `min_number_of=4` children, "max child progress" and "sum of all work" + * give different answers, and picking one is a product question). + * + * Only `all_of` and `min_number_of` (with a valid integer `operator_value`) + * are counted. Unknown or malformed operators contribute nothing and log + * a warning — we'd rather under-report than guess. + * + * For `min_number_of` operators, `completed` is capped at `operator_value` + * so extra electives don't inflate the overall total. + * + * Pass all top-level operators for overall program progress, or a single + * operator node for per-section progress. + */ +const getRequirementsProgress = ( + nodes: V2ProgramRequirement[], + courseEnrollments: Record, + programEnrollments: Record, +): { completed: number; total: number } => { + return nodes.reduce( + (acc, node) => { + if (node.data.node_type !== "operator") return acc + const children = node.children ?? [] + if (children.some((c) => c.data.node_type === "operator")) { + console.warn( + "getRequirementsProgress: nested operators are not supported and will be skipped", + ) + } + + const leaves = children.filter( + (c) => c.data.node_type === "course" || c.data.node_type === "program", + ) + const completed = leaves.filter((leaf) => + isLeafRequirementNodeCompleted( + leaf, + courseEnrollments, + programEnrollments, + ), + ).length + + if (node.data.operator === "all_of") { + return { + completed: acc.completed + completed, + total: acc.total + leaves.length, + } + } + + if (node.data.operator === "min_number_of") { + const minRequired = parseInt(node.data.operator_value ?? "", 10) + if (!isNaN(minRequired)) { + return { + completed: acc.completed + Math.min(completed, minRequired), + total: acc.total + minRequired, + } + } + } + + console.warn( + `getRequirementsProgress: unsupported operator "${node.data.operator}" (operator_value=${JSON.stringify(node.data.operator_value)}); skipping.`, + ) + return acc + }, + { completed: 0, total: 0 }, + ) +} + +export { getRequirementsProgress } From 60e298d7b3e4dffc43bdb046f2597b2c0f2e53a1 Mon Sep 17 00:00:00 2001 From: Zaman Afzal Date: Fri, 24 Apr 2026 12:15:43 +0500 Subject: [PATCH 4/5] feat: Adds Google Tag Manager (GTM) support (#3236) * feat: add GTM integration and runtime configuration --- env/frontend.env | 6 +++ env/frontend.local.example.env | 6 +++ env/shared.local.example.env | 7 ++++ frontends/main/src/app/layout.tsx | 65 +++++++++++++++++++++++++++++++ 4 files changed, 84 insertions(+) diff --git a/env/frontend.env b/env/frontend.env index 06e4af6639..0ca5e8be54 100644 --- a/env/frontend.env +++ b/env/frontend.env @@ -35,6 +35,12 @@ NEXT_PUBLIC_VERSION="local-dev" # Hubspot tracking - xprodev account for dev/RC environments NEXT_PUBLIC_HUBSPOT_PORTAL_ID=${HUBSPOT_PORTAL_ID} +# Google Tag Manager configured server-side (no NEXT_PUBLIC_*); values will be included in the GTM script/iframe URLs +GTM_TRACKING_ID=${GTM_TRACKING_ID} +GTM_AUTH=${GTM_AUTH} # NOTE: This is not a secret. +GTM_PREVIEW=${GTM_PREVIEW} +GTM_COOKIES_WIN=${GTM_COOKIES_WIN} + # OpenTelemetry tracing (server-side only — no NEXT_PUBLIC_ prefix needed) # These are read at runtime by the OTEL NodeSDK and injected by Kubernetes for # deployed environments. Sampling is disabled locally (0.0); set diff --git a/env/frontend.local.example.env b/env/frontend.local.example.env index f68006d4e8..22925ca0ae 100644 --- a/env/frontend.local.example.env +++ b/env/frontend.local.example.env @@ -1,2 +1,8 @@ NEXT_PUBLIC_EMBEDLY_KEY="" NEXT_PUBLIC_STAY_UPDATED_HUBSPOT_FORM_ID="" + +# Optional local GTM overrides (server-side runtime) +# GTM_TRACKING_ID= +# GTM_AUTH= +# GTM_PREVIEW= +# GTM_COOKIES_WIN= diff --git a/env/shared.local.example.env b/env/shared.local.example.env index 4fc78a8295..04405a6c3a 100644 --- a/env/shared.local.example.env +++ b/env/shared.local.example.env @@ -12,3 +12,10 @@ MITOL_API_DOMAIN=api.open.odl.local # dev only, should match domain of above # https://developers.google.com/recaptcha/docs/faq#id-like-to-run-automated-tests-with-recaptcha.-what-should-i-do # RECAPTCHA_SITE_KEY=6LeIxAcTAAAAAJcZVRqyHh71UMIEGNQ_MXjiZKhI # RECAPTCHA_SECRET_KEY=6LeIxAcTAAAAAGG-vFI1TnRWxMZNFuojJ4WifJWe + +# Google Tag Manager (server-side frontend runtime variables) +# GTM_TRACKING_ID= +# GTM_AUTH= +# GTM_PREVIEW= +# GTM_COOKIES_WIN= +# DEV values should be copied from GTM Environments UI. diff --git a/frontends/main/src/app/layout.tsx b/frontends/main/src/app/layout.tsx index db56a0c36f..cf45c40a02 100644 --- a/frontends/main/src/app/layout.tsx +++ b/frontends/main/src/app/layout.tsx @@ -10,6 +10,44 @@ import "./GlobalStyles" const NEXT_PUBLIC_ORIGIN = process.env.NEXT_PUBLIC_ORIGIN +const getGtmConfig = () => { + const trackingId = process.env.GTM_TRACKING_ID?.trim() + const auth = process.env.GTM_AUTH?.trim() + const preview = process.env.GTM_PREVIEW?.trim() + const cookiesWin = process.env.GTM_COOKIES_WIN?.trim() || "x" + + if (!trackingId || !auth || !preview) { + return null + } + + return { + trackingId, + auth, + preview, + cookiesWin, + } +} + +const buildGtmUrlQuery = ({ + trackingId, + auth, + preview, + cookiesWin, +}: { + trackingId: string + auth: string + preview: string + cookiesWin: string +}) => { + const params = new URLSearchParams({ + id: trackingId, + gtm_auth: auth, + gtm_preview: preview, + gtm_cookies_win: cookiesWin, + }) + return params.toString() +} + /** * As part of the root layout, this metadata object is site-wide defaults */ @@ -32,9 +70,25 @@ export default function RootLayout({ }: Readonly<{ children: React.ReactNode }>) { + const gtmConfig = getGtmConfig() + const gtmQuery = gtmConfig ? buildGtmUrlQuery(gtmConfig) : null + const gtmHeadScript = + gtmQuery !== null + ? `(function(w,d,s,l,q){w[l]=w[l]||[];w[l].push({'gtm.start': +new Date().getTime(),event:'gtm.js'});var f=d.getElementsByTagName(s)[0], +j=d.createElement(s),dl=l!=='dataLayer'?'&l='+l:'';j.async=true;j.src= +'https://www.googletagmanager.com/gtm.js?'+q+dl;f.parentNode.insertBefore(j,f); +})(window,document,'script','dataLayer',${JSON.stringify(gtmQuery)});` + : null + return ( + {gtmHeadScript ? ( + + ) : null} {/* Font files for Adobe neue haas grotesk. WARNING: This is linked to chudzick@mit.edu's Adobe account. @@ -51,6 +105,17 @@ export default function RootLayout({ /> + {gtmQuery ? ( +