From c9e39dcac0dd98359be36e078ee2df876fdf6aa1 Mon Sep 17 00:00:00 2001 From: shankar ambady Date: Wed, 25 Sep 2024 14:27:45 -0400 Subject: [PATCH 1/7] adding display label and migration to percolate queries --- .../0005_percolatequery_display_label.py | 22 +++++++++++++++++++ learning_resources_search/models.py | 6 +++++ 2 files changed, 28 insertions(+) create mode 100644 learning_resources_search/migrations/0005_percolatequery_display_label.py diff --git a/learning_resources_search/migrations/0005_percolatequery_display_label.py b/learning_resources_search/migrations/0005_percolatequery_display_label.py new file mode 100644 index 0000000000..0d049c6a87 --- /dev/null +++ b/learning_resources_search/migrations/0005_percolatequery_display_label.py @@ -0,0 +1,22 @@ +# 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, + null=True, + ), + ), + ] diff --git a/learning_resources_search/models.py b/learning_resources_search/models.py index 058381b2af..a9702a3939 100644 --- a/learning_resources_search/models.py +++ b/learning_resources_search/models.py @@ -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): From 07be562c1cfae1b5f5a1b034b561893682752f38 Mon Sep 17 00:00:00 2001 From: shankar ambady Date: Wed, 25 Sep 2024 14:28:48 -0400 Subject: [PATCH 2/7] regenerating specs --- frontends/api/src/generated/v1/api.ts | 6 ++++++ openapi/specs/v1.yaml | 4 ++++ 2 files changed, 10 insertions(+) diff --git a/frontends/api/src/generated/v1/api.ts b/frontends/api/src/generated/v1/api.ts index cf9b817ed5..62385b9658 100644 --- a/frontends/api/src/generated/v1/api.ts +++ b/frontends/api/src/generated/v1/api.ts @@ -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 } /** diff --git a/openapi/specs/v1.yaml b/openapi/specs/v1.yaml index af71655a91..1fe6d76a0e 100644 --- a/openapi/specs/v1.yaml +++ b/openapi/specs/v1.yaml @@ -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 From fd4eefb6c7a62e459e1dfff87529fa6e7de66942 Mon Sep 17 00:00:00 2001 From: shankar ambady Date: Wed, 25 Sep 2024 15:20:45 -0400 Subject: [PATCH 3/7] adding test and method for returning label --- learning_resources_search/models.py | 2 ++ learning_resources_search/models_test.py | 36 ++++++++++++++++++++++++ 2 files changed, 38 insertions(+) diff --git a/learning_resources_search/models.py b/learning_resources_search/models.py index a9702a3939..8b10a69981 100644 --- a/learning_resources_search/models.py +++ b/learning_resources_search/models.py @@ -43,6 +43,8 @@ def source_label(self): def source_description(self): channel = self.source_channel() + if self.display_label: + return self.display_label if channel: return channel.title return self.original_url_params() diff --git a/learning_resources_search/models_test.py b/learning_resources_search/models_test.py index fa070c6fe7..60144c001a 100644 --- a/learning_resources_search/models_test.py +++ b/learning_resources_search/models_test.py @@ -152,3 +152,39 @@ 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 +def test_percolate_query_display_labels(mocker, mocked_es): + """ + Test that makes sure we display the display label for a percolate query if it is defined + """ + mocker.patch( + "learning_resources_search.indexing_api.index_percolators", autospec=True + ) + mocker.patch( + "learning_resources_search.indexing_api._update_document_by_id", autospec=True + ) + ChannelFactory.create(search_filter="topic=Math", channel_type="topic") + ChannelFactory.create(search_filter="department=physics", channel_type="department") + ChannelFactory.create(search_filter="offered_by=mitx", channel_type="unit") + original_query = { + "q": "testing search filter", + "free": None, + "department": ["physics"], + "topic": ["math"], + "professional": None, + "certification": None, + "yearly_decay_percent": None, + } + test_label = "new courses about cats" + query = PercolateQueryFactory.create( + original_query=original_query, + query=original_query, + display_label=test_label, + ) + assert ( + query.original_url_params() + == "q=testing+search+filter&department=physics&topic=math" + ) + assert query.source_label() == "saved_search" From 5025437e89b83ac731d605235aa693667201c131 Mon Sep 17 00:00:00 2001 From: shankar ambady Date: Wed, 25 Sep 2024 15:41:10 -0400 Subject: [PATCH 4/7] fixing migration --- .../migrations/0005_percolatequery_display_label.py | 1 - 1 file changed, 1 deletion(-) diff --git a/learning_resources_search/migrations/0005_percolatequery_display_label.py b/learning_resources_search/migrations/0005_percolatequery_display_label.py index 0d049c6a87..a2c2814fc0 100644 --- a/learning_resources_search/migrations/0005_percolatequery_display_label.py +++ b/learning_resources_search/migrations/0005_percolatequery_display_label.py @@ -16,7 +16,6 @@ class Migration(migrations.Migration): blank=True, help_text="Friendly display label for the query", max_length=255, - null=True, ), ), ] From b30f09b7e22fbf5f3a5848bb1681f1e1531b2186 Mon Sep 17 00:00:00 2001 From: shankar ambady Date: Fri, 27 Sep 2024 12:44:20 -0400 Subject: [PATCH 5/7] adding display label to admin --- learning_resources_search/admin.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/learning_resources_search/admin.py b/learning_resources_search/admin.py index c0a6b23160..cf839aae59 100644 --- a/learning_resources_search/admin.py +++ b/learning_resources_search/admin.py @@ -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) From 686dcf40fe33305ed2ffac68b7731beae829af68 Mon Sep 17 00:00:00 2001 From: shankar ambady Date: Fri, 27 Sep 2024 12:49:00 -0400 Subject: [PATCH 6/7] adding test for source description --- learning_resources_search/models_test.py | 20 ++++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/learning_resources_search/models_test.py b/learning_resources_search/models_test.py index 60144c001a..7fe928f060 100644 --- a/learning_resources_search/models_test.py +++ b/learning_resources_search/models_test.py @@ -155,7 +155,11 @@ def test_percolate_query_search_labels(mocker, mocked_es): @pytest.mark.django_db -def test_percolate_query_display_labels(mocker, mocked_es): +@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 """ @@ -165,9 +169,10 @@ def test_percolate_query_display_labels(mocker, mocked_es): mocker.patch( "learning_resources_search.indexing_api._update_document_by_id", autospec=True ) - ChannelFactory.create(search_filter="topic=Math", channel_type="topic") - ChannelFactory.create(search_filter="department=physics", channel_type="department") - ChannelFactory.create(search_filter="offered_by=mitx", channel_type="unit") + + channel = ChannelFactory.create( + search_filter="offered_by=mitx", channel_type="unit" + ) original_query = { "q": "testing search filter", "free": None, @@ -188,3 +193,10 @@ def test_percolate_query_display_labels(mocker, mocked_es): == "q=testing+search+filter&department=physics&topic=math" ) 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() + ) From 40551bd4f171f5cf072d50c11cc6c9baef95e609 Mon Sep 17 00:00:00 2001 From: shankar ambady Date: Fri, 27 Sep 2024 13:54:07 -0400 Subject: [PATCH 7/7] fixing search conditionals in test --- learning_resources_search/models_test.py | 43 ++++++++++++++---------- 1 file changed, 25 insertions(+), 18 deletions(-) diff --git a/learning_resources_search/models_test.py b/learning_resources_search/models_test.py index 7fe928f060..3a6058e2ea 100644 --- a/learning_resources_search/models_test.py +++ b/learning_resources_search/models_test.py @@ -1,4 +1,5 @@ from types import SimpleNamespace +from urllib.parse import urlencode import pytest @@ -163,6 +164,15 @@ def test_percolate_query_display_labels( """ 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 ) @@ -170,29 +180,26 @@ def test_percolate_query_display_labels( "learning_resources_search.indexing_api._update_document_by_id", autospec=True ) - channel = ChannelFactory.create( - search_filter="offered_by=mitx", channel_type="unit" - ) - original_query = { - "q": "testing search filter", - "free": None, - "department": ["physics"], - "topic": ["math"], - "professional": None, - "certification": None, - "yearly_decay_percent": None, - } - test_label = "new courses about cats" + 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() - == "q=testing+search+filter&department=physics&topic=math" - ) - assert query.source_label() == "saved_search" + 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