Skip to content
Merged
Show file tree
Hide file tree
Changes from all 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
6 changes: 6 additions & 0 deletions frontends/api/src/generated/v1/api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3638,6 +3638,12 @@ export interface PercolateQuery {
* @memberof PercolateQuery
*/
source_type: SourceTypeEnum
/**
* Friendly display label for the query
* @type {string}
* @memberof PercolateQuery
*/
display_label?: string
}

/**
Expand Down
4 changes: 2 additions & 2 deletions learning_resources_search/admin.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,8 @@ class PercolateQueryAdmin(admin.ModelAdmin):
"""PercolateQuery Admin"""

model = models.PercolateQuery
list_display = ("original_query", "query")
search_fields = ("original_query", "query")
list_display = ("original_query", "query", "display_label")
search_fields = ("original_query", "query", "display_label")


admin.site.register(models.PercolateQuery, PercolateQueryAdmin)
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
# Generated by Django 4.2.16 on 2024-09-25 18:26

from django.db import migrations, models


class Migration(migrations.Migration):
dependencies = [
("learning_resources_search", "0004_alter_percolatequery_source_type"),
]

operations = [
migrations.AddField(
model_name="percolatequery",
name="display_label",
field=models.CharField(
blank=True,
help_text="Friendly display label for the query",
max_length=255,
),
),
]
8 changes: 8 additions & 0 deletions learning_resources_search/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,12 @@ class PercolateQuery(TimestampedModel):
source_type = models.CharField(
max_length=255, choices=[(choice, choice) for choice in SOURCE_TYPES]
)

display_label = models.CharField(
max_length=255,
blank=True,
help_text="Friendly display label for the query",
)
users = models.ManyToManyField(User, related_name="percolate_queries")

def source_label(self):
Expand All @@ -37,6 +43,8 @@ def source_label(self):

def source_description(self):
channel = self.source_channel()
Copy link
Member

Choose a reason for hiding this comment

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

Outside of this scope's PR changes, but should this function also take the source_type into consideration?

    def source_channel(self):
        original_query_params = self.original_url_params()
        channels_filtered = Channel.objects.filter(search_filter=original_query_params)
        if channels_filtered.exists() and self.source_type ==  CHANNEL_SUBSCRIPTION_TYPE:
            return channels_filtered.first()
        return None

Copy link
Contributor Author

Choose a reason for hiding this comment

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

potentially. - im currently working on another PR that involves sending actual emails for percolated searches so this might change there

if self.display_label:
return self.display_label
if channel:
return channel.title
return self.original_url_params()
Copy link
Member

Choose a reason for hiding this comment

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

Q: Should something else be displayed that's more user friendly than this when display_label and channel are both None?

Screenshot 2024-09-26 at 11 07 19 AM

Copy link
Member

Choose a reason for hiding this comment

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

One thing I found a bit counterintuitive but is outside the scope of this PR: the source_description is used as the title in this list while source_label is shown underneath that. I would expect the opposite.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

working on another pr for sending the emails I can see about changing this there.

Expand Down
55 changes: 55 additions & 0 deletions learning_resources_search/models_test.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
from types import SimpleNamespace
from urllib.parse import urlencode

import pytest

Expand Down Expand Up @@ -152,3 +153,57 @@ def test_percolate_query_search_labels(mocker, mocked_es):
== "q=testing+search+filter&department=physics&topic=math"
)
assert query.source_label() == "saved_search"


@pytest.mark.django_db
@pytest.mark.parametrize("is_channel_query", [True, False])
@pytest.mark.parametrize("test_label", ["new courses about cats", ""])
def test_percolate_query_display_labels(
mocker, mocked_es, test_label, is_channel_query
):
"""
Test that makes sure we display the display label for a percolate query if it is defined
"""

def encode_params(oparams):
ignore_params = ["endpoint"]
query = oparams
defined_params = {
key: query[key] for key in query if query[key] and key not in ignore_params
}
return urlencode(defined_params, doseq=True)

mocker.patch(
"learning_resources_search.indexing_api.index_percolators", autospec=True
)
mocker.patch(
"learning_resources_search.indexing_api._update_document_by_id", autospec=True
)

if is_channel_query:
original_query = {"department": ["physics"]}
channel = ChannelFactory.create(
search_filter=encode_params(original_query), channel_type="unit"
)
else:
original_query = {
"q": "testing search filter",
"certification": None,
"yearly_decay_percent": None,
}

query = PercolateQueryFactory.create(
original_query=original_query,
query=original_query,
display_label=test_label,
)
assert query.original_url_params() == encode_params(original_query)
if not is_channel_query:
assert query.source_label() == "saved_search"
assert query.source_description() == (
test_label
if test_label
else channel.title
if is_channel_query
else query.original_url_params()
)
4 changes: 4 additions & 0 deletions openapi/specs/v1.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -10106,6 +10106,10 @@ components:
query: {}
source_type:
$ref: '#/components/schemas/SourceTypeEnum'
display_label:
type: string
description: Friendly display label for the query
maxLength: 255
required:
- id
- original_query
Expand Down