Skip to content
Merged
Show file tree
Hide file tree
Changes from 14 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 18 additions & 0 deletions frontends/api/src/generated/v0/api.ts

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

18 changes: 18 additions & 0 deletions frontends/api/src/generated/v1/api.ts

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

60 changes: 60 additions & 0 deletions frontends/main/src/app-pages/SearchPage/SearchPage.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -583,9 +583,19 @@ describe("Search Page Tabs", () => {

test("Facet 'Offered By' uses API response for names", async () => {
const offerors = factories.learningResources.offerors({ count: 3 })

const resources = factories.learningResources.resources({
count: 3,
}).results

for (const [i, v] of resources.entries()) {
v.offered_by = offerors.results[i]
v.offered_by.display_facet = true
}
setMockApiResponses({
offerors,
search: {
results: resources,
metadata: {
aggregations: {
offered_by: offerors.results.map((o, i) => ({
Expand Down Expand Up @@ -618,6 +628,56 @@ test("Facet 'Offered By' uses API response for names", async () => {
expect(offeror2).toBeVisible()
})

test("Facet 'Offered By' only shows facets with 'display_facet' set to true", async () => {
const offerors = factories.learningResources.offerors({ count: 3 })

const resources = factories.learningResources.resources({
count: 3,
}).results

for (const [i, v] of resources.entries()) {
v.offered_by = offerors.results[i]
}
resources[0]!.offered_by!.display_facet = true
resources[1]!.offered_by!.display_facet = false
resources[2]!.offered_by!.display_facet = false

setMockApiResponses({
offerors,
search: {
results: resources,
metadata: {
aggregations: {
offered_by: offerors.results.map((o, i) => ({
key: o.code,
doc_count: 10 + i,
})),
},
suggestions: [],
},
},
})
renderWithProviders(<SearchPage />)
const showFacetButton = await screen.findByRole("button", {
name: /Offered By/i,
})

await user.click(showFacetButton)

const offeror0 = await screen.findByRole("checkbox", {
name: `${offerors.results[0].name} 10`,
})
const offeror1 = screen.queryByRole("checkbox", {
name: `${offerors.results[1].name} 11`,
})
const offeror2 = screen.queryByRole("checkbox", {
name: `${offerors.results[2].name} 12`,
})
expect(offeror0).toBeVisible()
expect(offeror1).not.toBeInTheDocument()
expect(offeror2).not.toBeInTheDocument()
})

test("Set sort", async () => {
setMockApiResponses({ search: { count: 137 } })

Expand Down
39 changes: 32 additions & 7 deletions frontends/main/src/page-components/SearchDisplay/SearchDisplay.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,8 @@ import {
ResourceCategoryEnum,
SearchModeEnumDescriptions,
} from "api"
import { useLearningResourcesSearch } from "api/hooks/learningResources"
import { learningResourceQueries } from "api/hooks/learningResources"
import { keepPreviousData, useQuery } from "@tanstack/react-query"
import { useAdminSearchParams } from "api/hooks/adminSearchParams"
import {
AvailableFacets,
Expand Down Expand Up @@ -568,12 +569,36 @@ const SearchDisplay: React.FC<SearchDisplayProps> = ({
facetNames,
page,
])

const { data, isLoading, isFetching } = useLearningResourcesSearch(
allParams as LRSearchRequest,
{ keepPreviousData: true },
)

const { data, isLoading, isFetching } = useQuery({
...learningResourceQueries.search(allParams as LRSearchRequest),
placeholderData: keepPreviousData,
select: (data) => {
// Handle missing data gracefully
if (!data.metadata.aggregations.offered_by || data.results.length === 0) {
return data
}
const offerors = Object.fromEntries(
data.results
.map((item) => item.offered_by)
.filter((value) => value && value.display_facet)
.map((value) => [value?.code, value]),
)

// only show offerors with display_facet set
return {
...data,
metadata: {
...data.metadata,
aggregations: {
...data.metadata.aggregations,
offered_by: data.metadata.aggregations.offered_by.filter(
(value) => value && value.key in offerors,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will leave review to @abeglova , but skimmed this and wanted to mention:

It's best to avoid in operator in JS, unfortunately. It dates from the dark(er) ages of JS (AI told me 1998) and has some odd behavior. For example, it checks membership in the object or object prototype, so

"hasOwnProperty" in {} // true 
"map" in [] // true
"filter" in [] // true 
"set" in new URLSearchParams() // true

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

woops python brain kicked in. I switched it to Object.keys().includes

),
},
},
}
},
})
const { data: user } = useUserMe()

const [mobileDrawerOpen, setMobileDrawerOpen] = React.useState(false)
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
# Generated by Django 4.2.25 on 2025-11-06 19:55

from django.db import migrations, models


class Migration(migrations.Migration):
dependencies = [
("learning_resources", "0099_alter_learningresource_resource_type"),
]

operations = [
migrations.AddField(
model_name="learningresourceofferor",
name="display_facet",
field=models.BooleanField(default=True),
),
]
2 changes: 2 additions & 0 deletions learning_resources/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -173,6 +173,8 @@ class LearningResourceOfferor(TimestampedModel):
more_information = models.URLField(blank=True)
# This field name means "value proposition"
value_prop = models.TextField(blank=True)
# whether or not to show this offeror as a facet in the UI
display_facet = models.BooleanField(default=True)

@cached_property
def channel_url(self):
Expand Down
2 changes: 1 addition & 1 deletion learning_resources/serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ class LearningResourceOfferorSerializer(serializers.ModelSerializer):

class Meta:
model = models.LearningResourceOfferor
fields = ("code", "name", "channel_url")
fields = ("code", "name", "channel_url", "display_facet")


class LearningResourceOfferorDetailSerializer(LearningResourceOfferorSerializer):
Expand Down
1 change: 1 addition & 0 deletions learning_resources/serializers_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -524,6 +524,7 @@ def test_content_file_serializer(settings, expected_types, has_channels):
"offered_by": {
"name": content_file.run.learning_resource.offered_by.name,
"code": content_file.run.learning_resource.offered_by.code,
"display_facet": True,
"channel_url": frontend_absolute_url(
f"/c/unit/{Channel.objects.get(unit_detail__unit=content_file.run.learning_resource.offered_by).name}/"
)
Expand Down
1 change: 1 addition & 0 deletions learning_resources_search/constants.py
Original file line number Diff line number Diff line change
Expand Up @@ -194,6 +194,7 @@ class FilterConfig:
"properties": {
"code": {"type": "keyword"},
"name": {"type": "keyword"},
"display_facet": {"type": "boolean"},
"channel_url": {"type": "keyword"},
},
},
Expand Down
6 changes: 6 additions & 0 deletions openapi/specs/v0.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -3379,6 +3379,8 @@ components:
type: string
readOnly: true
nullable: true
display_facet:
type: boolean
required:
- channel_url
- code
Expand Down Expand Up @@ -3435,6 +3437,8 @@ components:
maxLength: 200
value_prop:
type: string
display_facet:
type: boolean
required:
- channel_url
- code
Expand Down Expand Up @@ -3495,6 +3499,8 @@ components:
maxLength: 200
value_prop:
type: string
display_facet:
type: boolean
required:
- code
- name
Expand Down
6 changes: 6 additions & 0 deletions openapi/specs/v1.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -11626,6 +11626,8 @@ components:
type: string
readOnly: true
nullable: true
display_facet:
type: boolean
required:
- channel_url
- code
Expand Down Expand Up @@ -11682,6 +11684,8 @@ components:
maxLength: 200
value_prop:
type: string
display_facet:
type: boolean
required:
- channel_url
- code
Expand All @@ -11698,6 +11702,8 @@ components:
type: string
minLength: 1
maxLength: 256
display_facet:
type: boolean
required:
- code
- name
Expand Down
26 changes: 14 additions & 12 deletions vector_search/tasks.py
Original file line number Diff line number Diff line change
Expand Up @@ -381,7 +381,8 @@ def embeddings_healthcheck():
"""
Check for missing embeddings and summaries in Qdrant and log warnings to Sentry
"""
remaining_content_files = []

remaining_content_file_ids = []
remaining_resources = []
resource_point_ids = {}
all_resources = LearningResource.objects.filter(
Expand All @@ -405,10 +406,14 @@ def embeddings_healthcheck():
)
content_file_point_ids[point_id] = {"key": cf.key, "id": cf.id}
for batch in chunks(content_file_point_ids.keys(), chunk_size=200):
remaining_content_files.extend(
filter_existing_qdrant_points_by_ids(
batch, collection_name=CONTENT_FILES_COLLECTION_NAME
)
remaining_content_files = filter_existing_qdrant_points_by_ids(
batch, collection_name=CONTENT_FILES_COLLECTION_NAME
)
remaining_content_file_ids.extend(
[
content_file_point_ids.get(p, {}).get("id")
for p in remaining_content_files
]
)

for batch in chunks(
Expand All @@ -422,16 +427,13 @@ def embeddings_healthcheck():
)
)

remaining_content_file_ids = [
content_file_point_ids.get(p, {}).get("id") for p in remaining_content_files
]
remaining_resource_ids = [
resource_point_ids.get(p, {}).get("id") for p in remaining_resources
]
missing_summaries = _missing_summaries()
log.info(
"Embeddings healthcheck found %d missing content file embeddings",
len(remaining_content_files),
len(remaining_content_file_ids),
)
log.info(
"Embeddings healthcheck found %d missing resource embeddings",
Expand All @@ -442,20 +444,20 @@ def embeddings_healthcheck():
len(missing_summaries),
)

if len(remaining_content_files) > 0:
if len(remaining_content_file_ids) > 0:
_sentry_healthcheck_log(
"embeddings",
"missing_content_file_embeddings",
{
"count": len(remaining_content_files),
"count": len(remaining_content_file_ids),
"ids": remaining_content_file_ids,
"run_ids": set(
ContentFile.objects.filter(
id__in=remaining_content_file_ids
).values_list("run__run_id", flat=True)[:100]
),
},
f"Warning: {len(remaining_content_files)} missing content file "
f"Warning: {len(remaining_content_file_ids)} missing content file "
"embeddings detected",
)

Expand Down
Loading